ENH: Add API to manage view state in custom Axes. #4795

QuLogic · 2015-07-26T08:02:00Z

Custom Axes may require additional information than simply the x/y-limits to describe the view. This API will allow the navigation buttons on the toolbar to function correctly with such Axes (but not necessarily the zoom or pan buttons just yet.)

This PR is simply a quick-to-implement idea; I have not yet applied it anywhere, though the intended usage is in PolarAxes, of course. From discussions on #4699, it seems like something like this might be useful for mplot3d as well.

There's also the question of how to update the view. As we all know by now, setting the x/y-limits does not work if the axes are not Cartesian. But NavigationToolbar2.release_zoom does quite a bit of other stuff before setting the limits and I'm not sure how to reconcile this at the moment.

efiring · 2015-07-26T09:31:17Z

lib/matplotlib/axes/_base.py

+            default implementation saves the view limits. You *must* implement
+            :meth:`restore_view` if you implement this method.
+        """
+        raise NotImplementedError


It would be simpler to put the existing code for ordinary axes here instead of leaving these as NotImplementedError, wouldn't it? I don't see any disadvantage, but I'm probably missing something.

Probably; I'm more curious about the API at the moment.

efiring · 2015-07-26T19:21:12Z

attn: @OceanWolf, @fariza

fariza · 2015-07-26T21:53:41Z

I proposed something similar #2511 but it was rejected.
At the end, I added the "history" as a tool backend_tools.ToolViewsPositionsin the MEP22 implementation.
I would recommend to modify this tool to hold this information.
NavigationToolbar2 is going to be deprecated soon, I wouldn't invest too much effort to add functionality to it.

QuLogic · 2015-07-26T22:08:20Z

That was a slightly different proposal, though. I am not suggesting that the Axes store their own history, just that the history tool (wherever it is) asks the Axes what to store instead of assuming the view limits are sufficient.

fariza · 2015-07-26T23:32:32Z

I see... In that case why not call it get_view and set_view and move the default there instead of rising an exception?

QuLogic · 2015-07-26T23:52:10Z

get_ and set_ have an existing meaning that I'd like to avoid implying (like set_xlim/get_xlim); the "view" is supposed to be opaque to anything outside the Axes. Taking a cue from Transform, maybe something like freeze_view/unfreeze_view?

efiring · 2015-07-27T02:03:36Z

I like save/restore better than freeze/unfreeze; it's a better description of what is happening.

OceanWolf · 2015-07-27T10:58:33Z

What about push and pop? As I see it we basically deal with a view stack of states...

fariza · 2015-07-27T13:24:46Z

Regarding the names

save: Means to actually save something, in this case, the method is just returning a specific configuration (limits in the default case), it is not saving anything.
restore: means that it was this way before and we are putting it back, this is not true, the view can be completely made up
push and pop: implies that we are moving things inside a stack (this is not the case).
freeze and unfreeze: Similar to push and pop, it means that the state is stored inside the object. This is not the case.
get and set: You actually get the current configuration (view limits and stuff) and you actually set the configuration (view limits and stuff). Where you hold the information is another matter, it can be completely made up or in the most common case this information is stored inside the backend_tools.ToolViewsPositions object

If the methods are going to be inside the axes then, it should be possible to call them externally without having to go through the toolbar. In that case, they have to implement the default behavior and their names have to represent what it is actually doing.

OceanWolf · 2015-07-27T14:18:27Z

@fariza ahh yes, my mistake. we do use a stack, but the stack lies in the tool. So I agree that it looks the best out of the options presented, however I feel it a bit overkill if we generalise this... say in the future we had a state of 100 variables, then we store 100 changes even if we only changed one of them.

Maybe if the tool just stored a dictionary of the changes?

fariza · 2015-07-27T14:27:26Z

@OceanWolf The variables are stored inside the ToolViewsPositions.view dictionnary, so it should be transparent no matter what we are storing inside.

QuLogic · 2015-07-28T03:02:06Z

Now updated with get/set and an API for doing zoom-to-rect. It should now be possible for custom Axes to implement the zoom button just like they can implement the pan/zoom button.

QuLogic · 2015-07-28T03:03:06Z

lib/matplotlib/axes/_base.py

+
+        lastx, lasty, x, y = bbox
+
+        x0, y0, x1, y1 = original_view


I'm not really sure why it's necessary to use the frozen view here. Unlike pan/zoom mode, with zoom-to-rect, there's only one time that the view can change (when the button's released), so I wonder if I can just use the existing view limits here?

@tacaswell before merging this, can you confirm that this doesn't have any bad side effects?
I have tested it, and it works fine, but I'm not sure that frozen isn't necessary.

QuLogic · 2015-07-28T03:05:32Z

PS, is there an automated way of testing this sort of thing? I just ran examples/api/two_scales.py.

fariza · 2015-07-28T03:37:38Z

Unfortunately there is no way to automatically test.
With the upcoming MEP27 it will be easier to create a virtual backend and emulate user input. For the time being, just try it as much as you can.

QuLogic · 2015-07-28T05:53:52Z

If that's the case, I'm not sure why Travis failed, as it seems a bit unrelated.

jenshnielsen · 2015-07-28T09:01:53Z

The failure is unrelated. I restarted travis

fariza · 2015-07-28T15:58:50Z

lib/matplotlib/backend_tools.py

+            else:
+                continue
+
+            a.set_view_from_bbox((lastx, lasty, x, y), view, direction,


this view variable is not defined

fariza · 2015-07-28T23:09:33Z

@QuLogic Did you try it with PolarAxes as you mentionned in the description? I am curious to know if works as expected, or something else needs to be moved to Axes.
If possible, try it with both toolmanager and ToolbarNavigation2

QuLogic · 2015-07-28T23:39:26Z

I've been working on it here. It's a bit hacky and some things are buggy, but the API here seems to work well enough.

fariza · 2015-07-29T13:39:20Z

👍

fariza · 2015-07-31T12:58:22Z

@OceanWolf @tacaswell what do you guys think?

tacaswell · 2015-07-31T13:21:59Z

Sorry, I have my hands full else where and have not been paying attention to this thread at all.

efiring · 2015-07-31T18:07:55Z

Attn: @mdboom, @pelson: this is important, and I think it deals with
areas in which you are particularly knowledgeable.

On 8/1/15, Thomas A Caswell notifications@github.com wrote:

Sorry, I have my hands full else where and have not been paying attention to
this thread at all.

Reply to this email directly or view it on GitHub:
#4795 (comment)

pelson · 2015-08-03T17:20:21Z

Attn: @mdboom, @pelson: this is important, and I think it deals with areas in which you are particularly knowledgeable.

Thanks @efiring. The proposal makes a lot of sense - 👍 on the axes being able to control what and how the "view" is preserved. My only concerns are about:

how much logic went into set_view_bbox - I don't feel like that is something I'd like to repeat in a subclass, and yet I also don't know whether all of that method is useful to non-cartesian axes.
the names set_view and get_view. These sound like something that a user could see, and misuse accidentally - I'm not averse to having a private API for this behaviour (ax._get_axes_view / ax._set_axes_view would be fine IMO).

Otherwise, awesome contribution @QuLogic, thanks!

fariza · 2015-08-05T22:23:37Z

@pelson I actually prefer the public methods, they allow to easily switch views programatically.

pelson · 2015-08-05T22:45:18Z

they allow to easily switch views programatically

Completely. But imagine you were coming to mpl for the first time: there are already way to many public methods. This functionality is superb, and I know it will be well used by cartopy for one, but it isn't in the list of the first 30 methods on an Axes I'd show to a new user...

Alternatively, @shoyer and I talked about the concept of method namespaces such that you could do something like axes.gui.set_view() / axes.gui.get_view(). It seems that Pandas has an implementation of this already (and I have a hacked together proof of concept which I did before knowing about the pandas version.

tacaswell · 2015-08-05T22:48:38Z

@pelson Long term I would like to pull all of the plotting methods off of the Axes objects which will help address the issue of information overload. We can then use module-level name spacing to group related functions.

I really need to write up a long document of my thoughts on large-scale API changes.

fariza · 2015-08-05T22:49:48Z

@pelson Ok, I agree we have tooooooo many public methods in one place.
Sorry @QuLogic please change the names to private ones _set_view and _get_view

Custom Axes may require additional information than simply the x/y limits to describe the view. This API will allow the navigation buttons on the toolbar to function correctly with such Axes.

The main benefit here will be PolarAxes, but there might be other custom Axes that find this useful.

QuLogic · 2015-08-06T18:49:31Z

how much logic went into set_view_bbox - I don't feel like that is something I'd like to repeat in a subclass, and yet I also don't know whether all of that method is useful to non-cartesian axes.

Think of it this way: there were two copies in the two toolbars, and now there's only one copy in the axes, so I've reduced code redundancy!

QuLogic · 2015-08-06T19:16:43Z

I seem to always get this PS backend failure...

fariza · 2015-08-07T17:26:10Z

Is anybody against this?

ENH: Add API to manage view state in custom Axes.

pelson · 2015-08-27T06:31:08Z

@QuLogic - thanks for this awesome change 👍.
Would you mind submitting another PR to add an entry to the whats new? Thx

QuLogic · 2015-09-29T03:43:57Z

I'm pretty sure this is in the rc for 1.5, though it's tagged as 2.1.

jenshnielsen · 2015-09-29T08:45:29Z

@QuLogic Yes if you click a commit you can see which tags it's in. Both of these are in 1.5 rc1

QuLogic · 2015-09-29T21:47:58Z

Oh yes, I know; I just like to keep my milestones straight.

OceanWolf · 2015-09-29T22:38:56Z

Sorry for those just messaged, hit reply to wrong email, just delete.

tacaswell added the status: needs review label Jul 26, 2015

efiring reviewed Jul 26, 2015
View reviewed changes

tacaswell added this to the proposed next point release milestone Jul 26, 2015

QuLogic force-pushed the custom-axes-view branch from c6151a3 to 34d0143 Compare July 28, 2015 02:58

QuLogic reviewed Jul 28, 2015
View reviewed changes

OceanWolf mentioned this pull request Jul 28, 2015

MEP27 Decouple pyplot from backends (refactoring Gcf out of backend code) #4143

Closed

16 tasks

fariza reviewed Jul 28, 2015
View reviewed changes

QuLogic force-pushed the custom-axes-view branch from 34d0143 to 88afb19 Compare July 28, 2015 19:53

fariza mentioned this pull request Aug 3, 2015

Toolbars keep history if axes change (navtoolbar2 + toolmanager) #4857

Merged

QuLogic added 2 commits August 6, 2015 14:43

ENH: Add API to manage view state in custom Axes.

eb80fe5

Custom Axes may require additional information than simply the x/y limits to describe the view. This API will allow the navigation buttons on the toolbar to function correctly with such Axes.

ENH: Allow custom Axes to implement zoom-to-rect.

7e582b1

The main benefit here will be PolarAxes, but there might be other custom Axes that find this useful.

QuLogic force-pushed the custom-axes-view branch from 88afb19 to 7e582b1 Compare August 6, 2015 18:46

pelson added a commit that referenced this pull request Aug 27, 2015

Merge pull request #4795 from QuLogic/custom-axes-view

eb53398

ENH: Add API to manage view state in custom Axes.

pelson merged commit eb53398 into matplotlib:master Aug 27, 2015

tacaswell removed the status: needs review label Aug 27, 2015

QuLogic deleted the custom-axes-view branch August 29, 2015 21:39

This was referenced Aug 29, 2015

DOC: Add information on new views for custom Axes. #5010

Merged

Fixed ZoomPanBase to work with log plots #4970

Merged

QuLogic mentioned this pull request Sep 21, 2015

added None option to _get_view, also fixed a typo #5108

Merged

jenshnielsen modified the milestones: next point release (1.5.0), proposed next point release (2.1) Sep 30, 2015

ENH: Add API to manage view state in custom Axes. #4795

ENH: Add API to manage view state in custom Axes. #4795

Conversation

QuLogic commented Jul 26, 2015

efiring Jul 26, 2015

Choose a reason for hiding this comment

QuLogic Jul 26, 2015

Choose a reason for hiding this comment

efiring commented Jul 26, 2015

fariza commented Jul 26, 2015

QuLogic commented Jul 26, 2015

fariza commented Jul 26, 2015

QuLogic commented Jul 26, 2015

efiring commented Jul 27, 2015

OceanWolf commented Jul 27, 2015

fariza commented Jul 27, 2015

OceanWolf commented Jul 27, 2015

fariza commented Jul 27, 2015

QuLogic commented Jul 28, 2015

QuLogic Jul 28, 2015

Choose a reason for hiding this comment

fariza Aug 25, 2015

Choose a reason for hiding this comment

QuLogic commented Jul 28, 2015

fariza commented Jul 28, 2015

QuLogic commented Jul 28, 2015

jenshnielsen commented Jul 28, 2015

fariza Jul 28, 2015

Choose a reason for hiding this comment

fariza commented Jul 28, 2015

QuLogic commented Jul 28, 2015

fariza commented Jul 29, 2015

fariza commented Jul 31, 2015

tacaswell commented Jul 31, 2015

efiring commented Jul 31, 2015

pelson commented Aug 3, 2015

fariza commented Aug 5, 2015

pelson commented Aug 5, 2015

tacaswell commented Aug 5, 2015

fariza commented Aug 5, 2015

QuLogic commented Aug 6, 2015

QuLogic commented Aug 6, 2015

fariza commented Aug 7, 2015

pelson commented Aug 27, 2015

QuLogic commented Sep 29, 2015

jenshnielsen commented Sep 29, 2015

QuLogic commented Sep 29, 2015

OceanWolf commented Sep 29, 2015