Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Don't mark a patch transform as set if the parent transform is not set.#9426
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Is there a good test for this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just putting a reminder here to include an image test related to#9435
Sorry, changed my mind about where to target this to again. |
anntzer commentedOct 21, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
To be honest, looking at it again, I am not even that certain that that the constructor of that Shadow class makes a lot of sense. Specifically, if I got it correctly, the ox and oy values are first multiplied by the dots-per-point conversion factor (e.g. 100/72 at 100dpi), then interpreted as offsets indata coordinates. Which "of course(???...)" means that the shadow can move when you save the figure (as savefig.dpi does not have to equal figure.dpi) |
Looking at the history this goes back to JDH in 2004 so it pre-dates the transform machinery and I would guess added as a gee-whiz feature. If you think it could be cleaned up for 2.2 👍 |
anntzer commentedOct 21, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'm not really convinced of the utility of providing this class, but heh. A problem with interpreting ox, oy as offsets in data units is that the only two public APIs that rely on that class (other than Shadow itself) are So perhaps ox, oy should be interpreted either as axes coordinates or figure coordinates -- don't have a clear preference right now. Edit: I think I'll let@dstansby figure out how to handle the same problem with arrows and just copy his approach :-) |
f99506f
to71ec6c3
Compare@dstansby test has been added (a while ago, but no worries) |
@dstansby Would you like to re-review and merge if it is OK? |
When updating a patch from another artist, if the other artist'stransform is marked as "uninitialized" (`_transformSet == False`), thendon't mark the updatee's transform as set either.Typically, this occurs if the updator has not been added to an Axes yet;its transform will be set when that adding occurs. In that case, theupdatee's transform also needs to be set when actually being added to anAxes.
Seems ready to go in for 3.0 so labeling as RC. |
When updating a patch from another artist, if the other artist's
transform is marked as "uninitialized" (
_transformSet == False
), thendon't mark the updatee's transform as set either.
Typically, this occurs if the updator has not been added to an Axes yet;
its transform will be set when that adding occurs. In that case, the
updatee's transform also needs to be set when actually being added to an
Axes.
Closes the incorrect transform part of#9377.
PR Summary
PR Checklist