-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Correctly set formatters and locators on removed shared axis #14997
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
Conversation
Ok so I think this is a case of sharex=True meaning too many things. Are we garaunteeibg that sharing the axis means we are sharing the locators and formatters? That seems really hard to enforce and when you toss units into the mess, it becomes almost intractable. I agree that just removing an axis shouldn’t cause a crash, but I think the fundamental issue is that the locators and formatters were linked in the first place. |
At least IIRC @efiring has a fairly strong opinion that we do... |
isDefault = majfmt.axis.isDefault_majfmt | ||
axis.set_major_formatter(majfmt) | ||
if isDefault: | ||
majfmt.axis.isDefault_majfmt = True | ||
|
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.
This doesn't make any sense to me. You get the formatter for axis
and then you set the formatter for axis
to that formatter, and then you set the isDefault to the value axis had before. I don't see how this code is doing anything.
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 think the confusion here stems from the mappings of formatters to axis.
- Axis > Formatter is a one to one mapping
- Formatter > Axis is a one to many mapping
So a single Formatter
is linked to a single Axis
, but many other Axis
can make use of that Formatter
. I agree this is a nightmare to understand, but without re-doing the internals of how this works it is how it is.
So this code is getting the formatter that axis
is using, (that it might be borrowing) and then explicitly associating that formatter with axis
(so it is no longer borrowing it).
I appreciate this is messy code, but it is neccesitated by the weird way that
I think 2. is a bit much to do for |
fe858aa
to
1d71f21
Compare
Be good to rebase to double check that docs build, but I don't think that has anything to do w/ this PR. |
1d71f21
to
24d1837
Compare
Fixes #14911
The issue was that a shared axis might never have it's
Formatter/Locator
set, which is fine when it is shared, but when the axis storing theFormatter/Locator
it relies on is removed, the previous shared axis needs to have theFormatter/Locator
s set.Also fixed typo
_reset_axis_form
>_reset_axis_from
.I've tried a few things, but cannot come up with a more elegant solution than this; if anyone has any ideas suggestions welcome.