Toolbars keep history if axes change (navtoolbar2 + toolmanager) #4857
Conversation
|
@KevKeating have you seen #4795 ? |
|
I hadn't seen it, but I just took a look through it and it looks quite nice. It shouldn't be difficult to merge this PR with that one. Calls to xmin, xmax, ymin, ymax = cur_lim
a.set_xlim((xmin, xmax))
a.set_ylim((ymin, ymax))in |
|
I just fixed a PEP8 issue in a docstring that Travis found. There was also a Python 2.6 failure that I'll look into soon. |
|
I think the python 2.6 failure was just a random Travis failure. I have restarted the test |
6f39fd1 to
08219c2
Compare
|
Just updated this PR to use the new API from #4795. |
|
@tacaswell do you think it is pertinent just make the changes in toolmanager so no new features appear in the old ToolbarNavigation |
|
@fariza I think it is fine to only put this into toolmanager, but will defer final judgement to you on this. |
| # Define Home | ||
| self.push_current() | ||
| # Adding the clear method as axobserver, removes this burden from | ||
| # the backend | ||
| self.figure.add_axobserver(self.clear) |
There was a problem hiding this comment.
@KevKeating
If you add one axes, and don't call any of the navigation methods, your views and possitions will be out of sync.
Somebody accessing the views, positions...
Why don't leave the axobserver and instead of calling clear, calling update_home_views?
There was a problem hiding this comment.
@fariza
Good point. I'll add update_home_views as an axobserver. Should I remove the ToolbarNavigation changes or leave those in?
|
@KevKeating I really think it is better if we keep this confined to |
08219c2 to
92a6323
Compare
|
Just pushed an update. |
| # the backend | ||
| self.figure.add_axobserver(self.clear) | ||
|
|
||
| def clear(self, figure): |
There was a problem hiding this comment.
Don't remove the clear method, it could be useful for other tools.
Update it to include the update_home_views
There was a problem hiding this comment.
@KevKeating I think the clear method comment that I made is the last thing needed before merge.
There was a problem hiding this comment.
Sorry, that completely slipped my mind. I'll update the diff soon.
There was a problem hiding this comment.
I just added clear back in with
self.home_views[figure].clear()
self.update_home_views()added. That's the only change in this new commit.
92a6323 to
b5506dc
Compare
|
I'm not sure what's going on with the test failure. It looks like |
|
@KevKeating Do you think it is related to your changes? I don't see how. |
|
@fariza Me neither. I'm guessing that it's unrelated. |
Toolbars keep history if axes change (navtoolbar2 + toolmanager)
This is a fix for #2511. With this PR, the back, forward, and home buttons in the toolbar will continue to work even if the number of axes changes. I've implemented this fix in both the navtoolbar2 toolbar as well as the MEP22 toolmanager toolbar, so this PR essentially combines #3347 and fariza#5.
The PR adds a home_views dictionary that stores the initial views of all axes. If an axes object didn't exist when a view was created, then the initial view will be used for that axes in that view. Positions will only be restored if there is a position for all current axes objects. This avoids having newer axes overlap with older ones when clicking on back or home.
I've also removed toolbar.update() as an axobserver since it's no longer necessary. (I noticed that toolbar.update() was connected in all the backends except for Qt4, which explains the traceback I observed in #2511 when using Qt4.)