Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Emit xlim_changed on shared axes.#26011
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
| forotherinself._get_shared_axes(): | ||
| ifotherisnotself.axes: | ||
| other._axis_map[name]._set_lim( | ||
| v0,v1,emit=False,auto=auto) |
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.
could this just be solved with the one line change of:
# Emit callbacks on shared axes without recursingother.callbacks.process(f"{name}lim_changed",other)
inside of this for loop/if condition
seems overdesigned to introduce undocumented sentinel behavior to get one line to run.
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.
Ah yes, that'smuch better.
| ifemit: | ||
| other.callbacks.process(f"{name}lim_changed",other) |
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.
| ifemit: | |
| other.callbacks.process(f"{name}lim_changed",other) | |
| other.callbacks.process(f"{name}lim_changed",other) |
This if will never be false as it is guarded by the sameif emit: on line 1228 (with no overwriting of the variable.
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.
True, but I think guarding the shared-axes syncing withif emit: is actually wrong (#26085), whereas the extraif emit: here is correct, so perhaps let's keep it there so that we don't forget to put it in when removing the outer check?
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.
I was wondering a bit about that behavior, but didn't take the time to actually test it...
Could it be as simple as dedenting the for loop (and keeping this if)?
That would:
- call the code to actually update the shared axes, regardless of
emit - make
emittruly only tied to the callback behavior, which is what I would expect
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.
Dedenting the for loopwill lead to infinite recursion, so that will need some kind of marker to block recursion (but it can't be done by overloading the semantics of emit as I proposed initially...).
PR summary
Closes#15785 (see#15785 (comment)).
PR checklist