Skip to content

Simplify cla sharex/sharey code; alternative to #8710 #8720

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

Merged
merged 1 commit into from
Jun 11, 2017

Conversation

efiring
Copy link
Member

@efiring efiring commented Jun 4, 2017

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 9, 2017
@anntzer
Copy link
Contributor

anntzer commented Jun 10, 2017

Obviously we may be constrained by backcompat, but it is not clear to me why we'd expect cla() to reset the scales, but not the formatters and locators?

@efiring
Copy link
Member Author

efiring commented Jun 10, 2017

Notice that this is in the context of a shared axis, say the x-axis being shared between Axes A and B. You start by making A, setting its scale, locators, and formatters. Then you make B, sharing it with A. This calls B.cla(). What the code is doing is making B.xaxis.major refer to A.xaxis.major; these are Ticker instances, which are just references to a locator and a formatter (and the same for .minor). So this is the somewhat indirect and obscure way that the sharing of locators and formatters is done (contrary to a comment in the tickers module that locator instances should not be shared, if I remember correctly).
Next, before my PR, all the shared locators and formatters are copied.
Then the scale of the new B x-axis is set to match that of the A x-axis, using a call that also sets the shared locators and formatters to their defaults, even if they were changed on A from their defaults.
To fix that error, they are all restored from the saved copies.
In this PR I just eliminate the saving, the resetting to defaults, and the restoring.

But the short answer to your question is that in this context, with a shared axis, cla is not resetting the scale, it is just setting it to match that of the axes with which it is shared.

Now, this works correctly on initialization of B, but I just did a little test, calling B.cla() after plotting in A and B, and, with 2.0.2, it did not do the right thing--it preserved the x scale (which I had changed to log), but it reset the locator and formatter to the linear defaults. I think this is a bug, perhaps related to the earlier unconditional call to self.xaxis.cla() in B.cla().

@anntzer
Copy link
Contributor

anntzer commented Jun 10, 2017

OK, I understand your explanation.
Do you plan to address the later bug that you mention in this PR as well? Otherwise I think this can go in.

@efiring
Copy link
Member Author

efiring commented Jun 11, 2017

@anntzer I would prefer to have this PR go in as-is; I think it provides a little bit of cleanup with no side-effects (unless I am missing something), and is only peripherally related to the bug. I have been digging in to the sharing and aspect ratio business a bit, and the bug is just one part of that. I'm not sure whether it will make sense to fix it independently, or as part of a larger set of changes. I suspect the latter.

@tacaswell tacaswell merged commit b6eb043 into matplotlib:master Jun 11, 2017
@efiring efiring deleted the simplify_cla_share branch June 11, 2017 06:24
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.

3 participants