Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

DietmarSchwertberger
Copy link
Contributor

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 at https://matplotlib.org/users/navigation_toolbar.html
The unused and undocumented class SubplotToolWX has been removed (Google did not find any code using this.)

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/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@@ -1474,25 +1474,6 @@ def updateButtonText(self, lst):
}


class SubplotToolWX(wx.Frame):
Copy link
Contributor

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").

Copy link
Contributor Author

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")
Copy link
Contributor

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).

Copy link
Contributor Author

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 modified import NavigationToolbar2Wx as NavigationToolbar2WxAgg things are consistent again (no deprecation).

Not having the Toolbar alias in backend_wxagg can break things, though.

Copy link
Contributor Author

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 assignment Toolbar = NavigationToolbar2Wx with a derived and deprecated class. I would now be in favor of having this in wx and wxagg. What do you think?

Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, also for wxagg.

@DietmarSchwertberger DietmarSchwertberger added this to the v2.2.0 milestone Feb 20, 2018
@DietmarSchwertberger DietmarSchwertberger added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Feb 20, 2018
@DietmarSchwertberger
Copy link
Contributor Author

"Release critical" together with PR #10428, to have working wx backends.

@DietmarSchwertberger DietmarSchwertberger merged commit bd015d0 into matplotlib:master Feb 24, 2018
@DietmarSchwertberger DietmarSchwertberger deleted the wx-inconsistencies branch February 24, 2018 14:13
lumberbot-app bot pushed a commit that referenced this pull request Feb 24, 2018
dstansby added a commit that referenced this pull request Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: wx Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants