-
Notifications
You must be signed in to change notification settings - Fork 225
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
Tests are failing on latest matplotlib #426
Comments
I think this was caused by https://github.com/matplotlib/matplotlib/pull/22250/files
@anntzer I'm a little confused by what you are saying it is that other backends should do? Maybe part of the issue is that ipympl wasn't doing it this way to begin with? Should we be explicitly creating a toolbar in the figmanager init? The issue there is that I think we'd always first create a webagg toolbra, and then an ipympl toolbar, which seems unneccesary. or should we be defining the variables |
Sorry, I did not realize there would be subclasses of backend_webagg_core that did not support NavigationToolbar2WebAgg. Fortunately this is easy to fix: we can simply move the definition of _toolbar2_class (which third parties shouldn't have to mess with) to backend_webagg. Does diff --git i/lib/matplotlib/backends/backend_webagg.py w/lib/matplotlib/backends/backend_webagg.py
index bb18c993bd..7359ebdb7a 100644
--- i/lib/matplotlib/backends/backend_webagg.py
+++ w/lib/matplotlib/backends/backend_webagg.py
@@ -51,6 +51,10 @@ class FigureCanvasWebAgg(core.FigureCanvasWebAggCore):
pass
+class FigureManagerWebAgg(core.FigureManagerWebAgg):
+ _toolbar2_class = core.NavigationToolbar2WebAgg
+
+
class WebAggApplication(tornado.web.Application):
initialized = False
started = False
@@ -299,7 +303,7 @@ def ipython_inline_display(figure):
@_Backend.export
class _BackendWebAgg(_Backend):
FigureCanvas = FigureCanvasWebAgg
- FigureManager = core.FigureManagerWebAgg
+ FigureManager = FigureManagerWebAgg
@staticmethod
def show():
diff --git i/lib/matplotlib/backends/backend_webagg_core.py w/lib/matplotlib/backends/backend_webagg_core.py
index 8b5f1ba479..286305f1ca 100644
--- i/lib/matplotlib/backends/backend_webagg_core.py
+++ w/lib/matplotlib/backends/backend_webagg_core.py
@@ -453,7 +453,7 @@ class NavigationToolbar2WebAgg(backend_bases.NavigationToolbar2):
class FigureManagerWebAgg(backend_bases.FigureManagerBase):
- _toolbar2_class = ToolbarCls = NavigationToolbar2WebAgg
+ ToolbarCls = NavigationToolbar2WebAgg
def __init__(self, canvas, num):
self.web_sockets = set() fix the problem for you? (I left ToolbarCls which is now unused; it can be separately deprecated on Matplotlib's side. |
Unfortunately it does not, it shifts the error message to javascript as there is still something going wrong with the toolbar init ( possibly it's not being properly created?
|
@anntzer it seems that that patch results in the toolbar being %matplotlib widget
import matplotlib.pyplot as plt
import numpy as np
with plt.ioff():
fig = plt.figure()
plt.plot(np.sin(np.linspace(0, 20, 100)));
print(fig.canvas.toolbar is None) gives |
Ah, I see, you were relying on the ToolbarCls mechanism. I guess I need to have a proper deprecation for it, although that looks tricky :/ |
You open the browser console instructions for various browsers here: https://balsamiq.com/support/faqs/browserconsole/ |
oh! the reason it's not being displayed is probably because of the |
ah We can release an update with this fix soon (assuming compatible with old versions of mpl) - but would still be good to have a deprecation period. |
Can you quickly check the compat with older Matplotlibs? |
I can get this working on mpl main (with the above patch) and on |
maybe this (with the check on version) is better? diff --git a/ipympl/backend_nbagg.py b/ipympl/backend_nbagg.py
index 15813bb..e740cef 100644
--- a/ipympl/backend_nbagg.py
+++ b/ipympl/backend_nbagg.py
@@ -452,11 +452,13 @@ class Canvas(DOMWidget, FigureCanvasWebAggCore):
class FigureManager(FigureManagerWebAgg):
- ToolbarCls = Toolbar
+ if matplotlib.__version__ < "3.6":
+ ToolbarCls = Toolbar
def __init__(self, canvas, num):
FigureManagerWebAgg.__init__(self, canvas, num)
self.web_sockets = [self.canvas]
+ self.toolbar = Toolbar(self.canvas)
def show(self):
if self.canvas._closed:
|
Seems good to me. |
@ianhi Sorry I dropped the ball on this. Is there anything specific left to be done on Matplotlib's side (e.g. some deprecation? under does it need to be displayed?) (AFAICT mpl HEAD now works just fine even with ipympl 0.8.7 i.e. before fixing anything on your side.) |
I don't think you did? All seems well for the forseeable future
interesting. Did more things change after matplotlib/matplotlib#22454? Either way it seems that our tests will pick this up again in the future if it becomes an issue once more so I don't see a big need to make more changes given that everything currently works |
Mostly I thought I promised to add a deprecation path somewhere but didn't do it. |
Describe the issue
For example see: https://github.com/matplotlib/ipympl/actions/runs/1806282769
Getting a lot of these errors so something must have changed upstream w.r.t colorbars i suppose
The text was updated successfully, but these errors were encountered: