Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for normalized units in Matlab figures #1062

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jcmonteiro
Copy link

This PR should solve #819 and #340.

I ran the tests using runMatlab2TikzTests and got the same output as the one in the develop branch:

|            | Pass | Fail | Skip | Total |
| :--------- | ---: | ---: | ---: | ----: |
| Unreliable |    2 |   13 |    0 |    15 |
|   Reliable |   45 |   44 |    1 |    90 |
|      Total |   47 |   57 |    1 |   105 |

@egeerardyn
Copy link
Member

Thank you for your pul request.

I don't think this does everything for normalized units, because (if I remember correctly), those units are normalized with respect to the container of the object.
I.e. an axis with normalized width = 0.5 spans half the figure, a figure with normalized width = 0.5, spans half the screen (or even half the desktop possibly on a multi-screen setup, I reckon).

While this is already an improvement (it handles those situations in some cases, but not everywhere), I think it would be better to either implement the full thing (which means passing in the handle of the object into that function, writing a function to find the parent, doing the conversion) or at least keeping some kind of warning that the conversion might be incorrect.

Because I don't think either issue can be closed as-is.

@jcmonteiro
Copy link
Author

Oh, I didn't know that the normalized units option worked like this. In the way I implemented it, I guess the conversion is only going to work when converting a figure.

Do you think passing the handle, using this piece of code if it points to a figure, and otherwise issuing a warning is enough to get this PR merged?

@egeerardyn
Copy link
Member

As far as I'm concerned, that's fine indeed. But you might need to change quite a bit of code to pass in those handles. In any case; as long as it keeps warning the users that the conversion is not perfect, it's fine by me :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants