Skip to content

Regression in get_navigate_mode() return value #18150

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.

Sign up for GitHub

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

Jump to bottom
Closed
timhoffm opened this issue Aug 2, 2020 · 2 comments · Fixed by #18173
Closed

Regression in get_navigate_mode() return value #18150

timhoffm opened this issue Aug 2, 2020 · 2 comments · Fixed by #18173
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: toolbar
Milestone
v3.3.1

Comments

@timhoffm
Copy link
Member

timhoffm commented Aug 2, 2020 •
edited
Loading

Bug report

#17135 simplified pan/zoom toggling.

The values of the _Mode enum are returned via get_navigate_mode() (https://matplotlib.org/devdocs/api/_as_gen/matplotlib.axes.Axes.get_navigate_mode.html).

This is a breaking API change. Before the returned values were 'PAN', 'ZOOM', or None (as is still documented). Now, these values are 'pan/zoom', 'zoom rect' and ''. Ping @anntzer I don't see if these values translate 1:1. Why is _Mode.PAN = 'pan/zoom'?
@timhoffm timhoffm added this to the v3.3.1 milestone Aug 2, 2020
@timhoffm timhoffm added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Aug 2, 2020
@anntzer
Copy link
Contributor

anntzer commented Aug 2, 2020

That's just because I confused ax.{get,set}_navigate_mode() ('PAN'/'ZOOM'/None) and toolbar.mode ('pan/zoom'/'zoom rect'/''). I think this just needs something like

diff --git i/lib/matplotlib/backend_bases.py w/lib/matplotlib/backend_bases.py
index 2dd41cdf5..f42a7664f 100644
--- i/lib/matplotlib/backend_bases.py
+++ w/lib/matplotlib/backend_bases.py
@@ -2784,6 +2784,10 @@ class _Mode(str, Enum):
     def __str__(self):
         return self.value
 
+    @property
+    def _navigate_mode(self):
+        return {"NONE": None, "PAN": "PAN", "ZOOM": "ZOOM"}[self.name]
+
 
 class NavigationToolbar2:
     """
@@ -3047,7 +3051,7 @@ class NavigationToolbar2:
             self.mode = _Mode.PAN
             self.canvas.widgetlock(self)
         for a in self.canvas.figure.get_axes():
-            a.set_navigate_mode(self.mode)
+            a.set_navigate_mode(self.mode._navigate_mode)
         self.set_message(self.mode)
 
     def press_pan(self, event):
@@ -3117,7 +3121,7 @@ class NavigationToolbar2:
             self.mode = _Mode.ZOOM
             self.canvas.widgetlock(self)
         for a in self.canvas.figure.get_axes():
-            a.set_navigate_mode(self.mode)
+            a.set_navigate_mode(self.mode._navigate_mode)
         self.set_message(self.mode)
 
     def press_zoom(self, event):

?

@anntzer anntzer mentioned this issue Aug 2, 2020
Closed
@QuLogic
Copy link
Member

QuLogic commented Aug 4, 2020 •
edited
Loading

Went with a slightly different implementation. Too bad it isn't 'NONE' instead of None, though.

QuLogic added a commit to QuLogic/matplotlib that referenced this issue Aug 4, 2020
@QuLogic
Fix the return value of Axes.get_navigate_mode.
f4a75f8 
Fixes matplotlib#18150.
@QuLogic QuLogic mentioned this issue Aug 4, 2020
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: toolbar
Projects
None yet
Milestone
v3.3.1
Development

Successfully merging a pull request may close this issue.

Fix the return value of Axes.get_navigate_mode. QuLogic/matplotlib
3 participants
@QuLogic @anntzer @timhoffm