Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix Wx inconsistencies#10518
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
Fix Wx inconsistencies#10518
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -1474,25 +1474,6 @@ def updateButtonText(self, lst): | |||
| } | |||
| class SubplotToolWX(wx.Frame): | |||
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.
API changes (even for "apparently unused" stuff) should nearly always go through a deprecation period unless it's really too hard to keep around. Just decorate the class with@deprecated("2.2").
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.
Done
| @@ -71,15 +71,8 @@ def blit(self, bbox=None): | |||
| filetypes = FigureCanvasAgg.filetypes | |||
| @cbook.deprecated("2.2") | |||
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.
Please leave until 3.0 (one minor-release deprecation period at least, usually two).
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 don't see that any other NavigationToolbar2 is deprecated, especially not the wx non-Agg one.
With the derived class removed and the modifiedimport NavigationToolbar2Wx as NavigationToolbar2WxAgg things are consistent again (no deprecation).
Not having the Toolbar alias in backend_wxagg can break things, though.
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.
Having a deprecation for the Toolbar alias would require to replace the assignmentToolbar = NavigationToolbar2Wx with a derived and deprecated class. I would now be in favor of having this in wx and wxagg. What do you think?
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.
derived deprecated class sounds good to me.
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.
Thanks. Done, incl. alternatives in deprecation messages.
| from .backend_wx import( | ||
| _BackendWx, _FigureCanvasWxBase, FigureFrameWx, NavigationToolbar2Wx) | ||
| from .backend_wx import_BackendWx, _FigureCanvasWxBase, FigureFrameWx | ||
| from .backend_wx import NavigationToolbar2Wx as NavigationToolbar2WxCairo |
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.
can go in the previous line (would prefer this) as
from .backend_wx import ( _BackendWx, _FigureCanvasWxBase, FigureFrameWx, NavigationToolbar2Wx as NavigationToolbar2WxCairo)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.
Done, also for wxagg.
"Release critical" together with PR#10428, to have working wx backends. |
Backport PR#10518 on branch v2.2.x
PR Summary
From discussion at PR 10428:
The wx examples in master are currently not consistent with the recent changes around NavigationToolbar2.
This PR makes NavigationToolbar2 consistent through the three wx backends.
Examples are adjusted to be similar to the documentation example athttps://matplotlib.org/users/navigation_toolbar.html
The unused and undocumented class
SubplotToolWXhas been removed (Google did not find any code using this.)PR Checklist