NavigationToolbar breaks if axes are added during use.

The NavigationToolbar saves the history of view limits in a stack. Each time the view is changed, the toolbar pushes the view limits for each axes onto the stack. Each time you go back it pops out the last view limits for each axes.

If you change the number of axes in-between changes to the view limits, then the number of pops won't match the number of pushes (or at least they won't correspond to the correct axes).

Maybe the view stack could be stored on each axes object. Alternatively, the toolbar could store a dictionary mapping axes to view stacks (this might require some manual clean up if axes are deleted).

See also PR #1849.

To reproduce:

plt.subplot(211)
plt.plot([0, 1])

# Pan or zoom the plot.
# If you press the Home button now, you'll return to the original view limits.

plt.subplot(212)

# If you press the Home button now, nothing will happen

@farzia

@farzia Does mep22 address this issue?

@tacaswell

@tacaswell for the moment, it doesn't address this problem.
I just moved the stack to navigation.
I like @tonysyu idea of moving the stack to the axes, this will also make sense if we implement the possibility to change the figure from one figure manager to another (MEP23 maybe?... multi figure manager), so the history follows the axes.

ok, removed the label.

@tacaswell

@tacaswell I am cleaning the MEP22 and I think it is convenient to move views and positions from Navigation to the axes object.
I was thinking to add these two inside axes._base._AxesBase
Is there any reason these won't belong there?

Adding the following attributes

views
positions

and the following methods

update_view
update_position
push_current_view
push_current_position

I think the argument to keep the stack in the navigation/toolbar objects is that that is where the buttons are. I am a little worried about the inter-layer coupling that this would add.

I forget, is Navigation optional? If it is, then I think all of the logic should stay in it, if it is mandatory then I think pushing this to the Axes object is fine.

After reviewing again the problem, I think this is not a bug (it's a feature ;) ).
The fact that nothing happens when pressing Home after second subplot is added, is normal, it is the desired behavior. If not, it would be impossible to keep the views and positions in sync between different axes.
That is why the toolbar.update method is added as axobserver by the backends.

I think this should be marked as invalid. Unless we want to change the behavior...

This issue can lead to a traceback in matplotlib 1.3.1. To reproduce the traceback using PyQt4, run the following from a Python terminal

from matplotlib.backends.backend_qt4agg import FigureCanvasQTAgg as FigureCanvas
from matplotlib.figure import Figure
from matplotlib.backends.backend_qt4agg import NavigationToolbar2QTAgg as NavToolbarQt
from PyQt4 import QtGui, QtCore
from matplotlib import cm
import numpy

app = QtGui.QApplication([])
widget = QtGui.QWidget()
fig = Figure()
canvas = FigureCanvas(fig)
canvas.setParent(widget)
plot = fig.add_subplot(111)
scatter = plot.scatter([1, 2], [3, 4], cmap=cm.spring, color="red")
scatter.set_array(numpy.array([5, 6]))
toolbar = NavToolbarQt(canvas, widget, True)
layout = QtGui.QVBoxLayout(widget)
layout.addWidget(toolbar)
layout.addWidget(canvas)
widget.show()

In the matplotlib window that appears, use either the pan or the zoom tool to change the view. Then run the following into the Python terminal:

cb = fig.colorbar(scatter)
fig.subplots_adjust()
canvas.draw()

Then click the home button in the matplotlib window.

This produces the following traceback

Traceback (most recent call last):
  File "C:\Programs\Anaconda\lib\site-packages\matplotlib\backend_bases.py", line 2745, in home
    self._update_view()
  File "C:\Programs\Anaconda\lib\site-packages\matplotlib\backend_bases.py", line 3142, in _update_view
    xmin, xmax, ymin, ymax = lims[i]
IndexError: list index out of range

I'd be happy to file this as a separate bug if desired, although it seems to have an identical cause to the issue described by tonysyu. I don't think that having Home do nothing (or cause a traceback) is a desired behavior in this scenario. Ideally, it should be possible to reset all axes to their original views without throwing off the limit and position stacks.

One potential solution is to store dictionaries in the NavigationToolbar2._views() stack rather than lists. (This solution is based on the 1.3.1 source code, although it sounds like it would still be relevant to MEP22 based on fariza's comments above.) Each dictionary would map an axes to a tuple of (xmin, xmax, ymin, ymax). There could then be a separate dictionary (NavigationToolbar2._home_views) that would store "home views" for each axes. The _home_views dictionary could be updated in both push_current() and _update_view(). Updating _home_views involves making sure that there in an entry for all current axes. If there is no entry, then one is added with the current (xmin, xmax, ymin, ymax) for that axes.

Then, if _update_view() tries to restore a view that doesn't contain data for an axes, it can use the _home_views entry for that axes instead. This way, Home will always restore all axes to their original views, and the Back and Forward buttons will properly affect all axes that were present when the specified view was created. The only additional bookkeeping would be the _home_views dictionary.

I haven't looked into positions much, but I'm guessing it would be best to not modify positions unless all axes are found in the current NavigationToolbar2._positions frame, since otherwise it might lead to overlapping axes.

I can work on a pull request that implements this, although I'm not sure of the best place to do this. Would it be better to base any work on the current master branch or off of MEP22?

@fariza

I just skimmed this, but my quick reaction to your last question is to base any new work off of @fariza 's MEP22 work. I am wary of touching too much of the GUI stack for 1.4 due to time (it should be released 'soon') and getting the MEP22 work merged is one of the top priorities for 1.5.

I have no experience with QT, but I see that you are not connecting the typical

def notify_axes_change(fig):
            # This will be called whenever the current axes is changed
            if self.toolbar is not None:
                self.toolbar.update()
self.canvas.figure.add_axobserver(notify_axes_change)

The toolbar.update() clears the views and positions (history) whenever a new axes is added or deleted.

The problem with using update, is that we get rid of history just for one axes creation or deletion.
In my own experience this is not a problem, but it might be for somebody else.
Keeping a separate record for each axes is a reasonable solution, but again, what happens when back and forward are used and the axes have different history?

@fariza

@fariza, I didn't realize that was a typical function when hooking up the toolbar, but that makes sense and would definitely avoid the traceback. My proposed solution wouldn't keep a separate record for each axes, so keeping things in sync when back or forward are clicked shouldn't be an issue. Instead, there'd be an additional dictionary containing the "home view" for each axes. Whenever _update_view tries to restore a view and is missing data for an axes, it would use the "home view" for that axes (but still use the "correct" view for all other axes).

@KevKeating

@KevKeating The PR would be great, but I propose to wait for the MEP22 to be fully discussed and merged.

For the record, in MEP22, I removed the need to add update method as axobserver from the backend. The tools do this automatically.

The PRs solved the problem

Navigation Menu

Repository navigation

NavigationToolbar breaks if axes are added during use. #2511

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

NavigationToolbar breaks if axes are added during use. #2511

Description

Activity

tacaswell commented on Jan 22, 2014

fariza commented on Jan 22, 2014

tacaswell commented on Jan 22, 2014

fariza commented on Feb 5, 2014

tacaswell commented on Feb 5, 2014

fariza commented on Feb 6, 2014

KevKeating commented on May 27, 2014

tacaswell commented on May 27, 2014

fariza commented on May 29, 2014

KevKeating commented on May 29, 2014

fariza commented on May 29, 2014

fariza commented on Jul 25, 2014

fariza commented on May 13, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions