Emit xlim_changed on shared axes. #26011
Conversation
| for other in self._get_shared_axes(): | ||
| if other is not self.axes: | ||
| other._axis_map[name]._set_lim( | ||
| v0, v1, emit=False, auto=auto) |
There was a problem hiding this comment.
could this just be solved with the one line change of:
# Emit callbacks on shared axes without recursing
other.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.
Ah yes, that's much better.
| if emit: | ||
| other.callbacks.process(f"{name}lim_changed", other) |
There was a problem hiding this comment.
| if emit: | |
| 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 same if emit: on line 1228 (with no overwriting of the variable.
There was a problem hiding this comment.
True, but I think guarding the shared-axes syncing with if emit: is actually wrong (#26085), whereas the extra if 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.
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.
Dedenting the for loop will 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