-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
NavigationToolbar breaks if axes are added during use. #2511
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
Comments
@farzia Does mep22 address this issue? |
@tacaswell for the moment, it doesn't address this problem. |
ok, removed the label. |
@tacaswell I am cleaning the MEP22 and I think it is convenient to move Adding the following attributes
and the following methods
|
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 |
After reviewing again the problem, I think this is not a bug (it's a feature ;) ). 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
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 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, 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? |
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
The The problem with using update, is that we get rid of history just for one axes creation or deletion. |
@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 |
@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 |
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:
The text was updated successfully, but these errors were encountered: