Skip to content
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

Saving png figures matplotlib 3.3.0 results in "AttributeError: __delete__" #252

Closed
tobiasjj opened this issue Jul 31, 2020 · 21 comments · Fixed by #264
Closed

Saving png figures matplotlib 3.3.0 results in "AttributeError: __delete__" #252

tobiasjj opened this issue Jul 31, 2020 · 21 comments · Fixed by #264

Comments

@tobiasjj
Copy link

Describe the issue

When trying to save png figures with ipympl, interactive widgets, and the latest matplotlib (3.3.0), the saving fails with the following error message:

import matplotlib
matplotlib.use('module://ipympl.backend_nbagg')

from matplotlib import pyplot as plt
fig = plt.figure()
fig.savefig('test.png')

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-c4b3ac84131c> in <module>
      6 from matplotlib import pyplot as plt
      7 fig = plt.figure()
----> 8 fig.savefig('test.png')

/usr/local/lib/python3.8/dist-packages/matplotlib/figure.py in savefig(self, fname, transparent, **kwargs)
   2309                 patch.set_edgecolor('none')
   2310 
-> 2311         self.canvas.print_figure(fname, **kwargs)
   2312 
   2313         if transparent:

/usr/local/lib/python3.8/dist-packages/matplotlib/backend_bases.py in print_figure(self, filename, dpi, facecolor, edgecolor, orientation, format, bbox_inches, pad_inches, bbox_extra_artists, backend, **kwargs)
   2223                 self.figure.set_edgecolor(origedgecolor)
   2224                 self.figure.set_canvas(self)
-> 2225             return result
   2226 
   2227     @classmethod

/usr/lib/python3.8/contextlib.py in __exit__(self, type, value, traceback)
    118         if type is None:
    119             try:
--> 120                 next(self.gen)
    121             except StopIteration:
    122                 return False

/usr/local/lib/python3.8/dist-packages/matplotlib/cbook/__init__.py in _setattr_cm(obj, **kwargs)
   2064         for attr, orig in origs.items():
   2065             if orig is sentinel:
-> 2066                 delattr(obj, attr)
   2067             else:
   2068                 setattr(obj, attr, orig)

AttributeError: __delete__

Saving figures as pdf or pngfigures with an older Matplotlib version (3.2.0) works well.

Versions

Matplotlib version: 3.3.0

python -c "import sys; print('\n',sys.version); import ipympl; print('ipympl version:', ipympl.__version__)" && jupyter --version && jupyter nbextension list && jupyter labextension list

-->

3.8.5 (default, Jul 20 2020, 18:32:44)
[GCC 9.3.0]
ipympl version: 0.5.7
jupyter core     : 4.6.3
jupyter-notebook : 6.0.3
qtconsole        : 4.7.4
ipython          : 7.16.1
ipykernel        : 5.3.4
jupyter client   : 6.1.6
jupyter lab      : 2.2.2
nbconvert        : 5.6.1
ipywidgets       : 7.5.1
nbformat         : 5.0.7
traitlets        : 4.3.3
Known nbextensions:
  config dir: /usr/etc/jupyter/nbconfig
    notebook section
      jupyter-js-widgets/extension disabled
  config dir: /usr/local/etc/jupyter/nbconfig
    notebook section
      bqplot/extension  enabled
      - Validating: OK
      itkwidgets/extension  enabled
      - Validating: OK
      jupyter-datawidgets/extension  enabled
      - Validating: OK
      jupyter-leaflet/extension  enabled
      - Validating: OK
      jupyter-matplotlib/extension  enabled
      - Validating: OK
      jupyter-js-widgets/extension  enabled
      - Validating: OK
  config dir: /etc/jupyter/nbconfig
    notebook section
      jupyter-js-widgets/extension  enabled
      - Validating: OK
JupyterLab v2.2.2
Known labextensions:
   app dir: /usr/local/share/jupyter/lab
        @jupyter-widgets/jupyterlab-manager v2.0.0  enabled  OK
        @jupyterlab/hub-extension v2.0.2  enabled  OK
        jupyter-matplotlib v0.7.3  enabled  OK
        jupyterlab-datawidgets v6.3.0  enabled  OK
@scimax
Copy link

scimax commented Aug 3, 2020

I experienced the same issue. As I thought it were related to #115 I posted my config there. But yes, the exception is different, so I should have considered it as a different issue. Thanks for opening it!

@ianhi
Copy link
Collaborator

ianhi commented Aug 13, 2020

this is also a problem if you display(fig).

I think is an important bug. That error is a bit opaque to me though. @tacaswell @martinRenou @thomasaarholt do any of you have thoughts on where to start here?

@martinRenou
Copy link
Member

I am not sure. I find this odd that it worked on 3.2 and not 3.3. Nothing changed Python side between those two versions. It might be a Python dependency change.

@ianhi did you investigate on what this function does and why it looks for this delete attribute?

@ianhi
Copy link
Collaborator

ianhi commented Aug 24, 2020

I have not, I got confused by all the context managers/wrappers that were happening. Though looking at it again briefly it looks as though there was a recent commit to the setattr_cm function that is in 3.3.x but not in 3.2.x https://github.com/matplotlib/matplotlib/blame/079e015ccad39526098098c05b5e7e510fef19df/lib/matplotlib/cbook/__init__.py#L2071

matplotlib/matplotlib#17620

Though what difference that would make to this backend and not others is beyond me.

@ianhi
Copy link
Collaborator

ianhi commented Aug 24, 2020

Also from a few lines up there is this comment (ref)

        # otherwise this is _something_ we are going to shadow at
        # the instance dict level from higher up in the MRO.  We
        # are going to assume we can delattr(obj, attr) to clean
        # up after ourselves.  It is possible that this code will
        # fail if used with a non-property custom descriptor which
        # implements __set__ (and __delete__ does not act like a
        # stack).  However, this is an internal tool and we do not
        # currently have any custom descriptors.

So I guess being a subclass of HasTraits is causing this problem - because (please correct if I'm wrong) traitlets is a custom descriptor that implements __set__. If the problem is that function deep in matplotlib interacting poorly with traitlets then maybe matplotlib proper is the correct place to fix the bug?

@martinRenou
Copy link
Member

Maybe, I don't really know.

@tacaswell do you know what this code is and how we should properly fix it? Would a try/catch ignoring the error be enough?

@ianhi
Copy link
Collaborator

ianhi commented Aug 25, 2020

Did a bit of bisecting:
At matplotlib/matplotlib@9e88fa5 there is an error with set
image
and with matplotlib/matplotlib@2706fa6 the error changes to be about delete
image

@tacaswell
Copy link
Member

tacaswell commented Aug 25, 2020

Agree that this is a critical bug.

@ianhi Please always prefer to post text rather than screen shots.

This does need to be fixed on the mpl side, but we need some help from someone who understands why our attempts to identify descriptors is failing with traitlets.

https://github.com/matplotlib/matplotlib/blob/bb71b00ac1a16c41054c2a5907d04f65da3e8565/lib/matplotlib/cbook/__init__.py#L2044-L2085 is the code in question.

We are monkey patching attributes internally (sometimes just resetting normal attributes, sometimes shadowing methods) and we want to safely reset it back to what it was before. In same cases we need to do a setattr (things that live in the instance __dict__, @propertys and apparently traitlets) and in other cases we need to del the attribute (because we are shadowing something from the class dictionary or a method).

Very specifically, this logic:
https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/cbook/__init__.py#L2053-L2074

        orig = getattr(obj, attr, sentinel)
        if attr in obj.__dict__ or orig is sentinel:
            # if we are pulling from the instance dict or the object
            # does not have this attribute we can trust the above
            origs[attr] = orig
        else:
            # if the attribute is not in the instance dict it must be
            # from the class level
            cls_orig = getattr(type(obj), attr)
            # if we are dealing with a property (but not a general descriptor)
            # we want to set the original value back.
            if isinstance(cls_orig, property):
                origs[attr] = orig
            # otherwise this is _something_ we are going to shadow at
            # the instance dict level from higher up in the MRO.  We
            # are going to assume we can delattr(obj, attr) to clean
            # up after ourselves.  It is possible that this code will
            # fail if used with a non-property custom descriptor which
            # implements __set__ (and __delete__ does not act like a
            # stack).  However, this is an internal tool and we do not
            # currently have any custom descriptors.
            else:
                origs[attr] = sentinel

is not correctly identifying traitlets, how to we fix that?

There was a more general version of the code that went in at matplotlib/matplotlib#17561

@ianhi
Copy link
Collaborator

ianhi commented Sep 5, 2020

who understands why our attempts to identify descriptors is failing with traitlets.

Unfortunately, I don't qualify for this. But maybe @martinRenou would know who to ask?

is not correctly identifying traitlets, how to we fix that?

Could that code check if the ipympl backend is in use, and if so use the version that was in 3.2? Obviously this is not an ideal, or even good, solution. But this bug basically makes it impossible to upgrade beyond 3.2 when using ipympl which feels worse.

Alternatively, would it be reasonable for traitlets to implement __delete__?

Minimal reproduction with only traitlets:

from traitlets import HasTraits, Unicode

class Identity(HasTraits):
    username = Unicode()
ident = Identity()
delattr(ident, 'username')

gives:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-13-0624f32acdce> in <module>
----> 1 delattr(ident, 'username')

AttributeError: __delete__

@martinRenou
Copy link
Member

martinRenou commented Sep 7, 2020

Alternatively, would it be reasonable for traitlets to implement delete?

Traitlets gets updated at a very slow pace, we might want to monkeypatch it in ipympl, doing something like:

from traitlets import HasTraits as OrigHasTraits

class HasTraits(OrigHasTraits):
    def __delete__(self, attribute):
        pass

I wonder what this __delete__ should do though, we won't be able to delete traits, other attributes should be fine.

@ianhi
Copy link
Collaborator

ianhi commented Sep 7, 2020

we won't be able to delete traits, other attributes should be fine.

Unfortunately, I think this may be primarily a problem with traits. If I extend the example it looks as though it only happens with traits:

from traitlets import HasTraits, Unicode

class Identity(HasTraits):
    username = Unicode()
ident = Identity()
#no errors
setattr(ident, 'test-attr', 'test')
delattr(ident,'test-attr')

# errors
delattr(ident, 'username')
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-17-f01352160376> in <module>
      9 
     10 # errors
---> 11 delattr(ident, 'username')

AttributeError: __delete__

Any idea which attributes are being accessed and deleted by setattr_cm?

@ianhi
Copy link
Collaborator

ianhi commented Sep 7, 2020

I added a print statement to setattr_cm:

diff --git a/lib/matplotlib/cbook/__init__.py b/lib/matplotlib/cbook/__init__.py
index 2b52dd414..da3d5c24a 100644
--- a/lib/matplotlib/cbook/__init__.py
+++ b/lib/matplotlib/cbook/__init__.py
@@ -2091,6 +2091,7 @@ def _setattr_cm(obj, **kwargs):
     finally:
         for attr, orig in origs.items():
             if orig is sentinel:
+                print(attr)
                 delattr(obj, attr)
             else:
                 setattr(obj, attr, orig)

and it looks as though attribute in question is _is_saving which is a traitlets Bool:

_is_saving = Bool()

This is true for both plt.savefig and display(fig). Although when you display(fig) several other attributes get set and deleted before the error occurs:

draw
close_group
draw_path
draw_path_collection
draw_tex
draw_text
open_group
_is_saving # error on this one

@tacaswell
Copy link
Member

Implementing __delete__ on the traits will eliminate the error, but make the context manager behave incorrectly (as it will not reset the value back to what it was). The fix has to be identifying the descritpors and handling them as needed in Matplotlib.

@ianhi
Copy link
Collaborator

ianhi commented Sep 7, 2020

@tacaswell sorry I keep diverting away from the final solution of implementing better handling in matplotlib, unfortunately I don't think I can help there. But! I do have a workaround that can be implemented in ipympl that seems to give all the correct behaviors and I don't think will messing with the context manager:

It turns out that _is_saving only shows up in one place in the codebase: https://github.com/matplotlib/ipympl/search?q=_is_saving&unscoped_q=_is_saving Most importantly, it is not used at all on the javascript side so I don't think it actually needs to be defined using traitlets. So if you delete the line defining it using traitlets then everything works as expected because it inherits _is_saving from its super class:

diff --git a/ipympl/backend_nbagg.py b/ipympl/backend_nbagg.py
index de37bab..8b88609 100644
--- a/ipympl/backend_nbagg.py
+++ b/ipympl/backend_nbagg.py
@@ -199,7 +199,6 @@ class Canvas(DOMWidget, FigureCanvasWebAggCore):
     _current_image_mode = Unicode()
     _dpi_ratio = Float(1.0)
     _is_idle_drawing = Bool()
-    _is_saving = Bool()
     _button = Any()
     _key = Any()
     _lastx = Any()

Which allows

%matplotlib widget
import matplotlib
from matplotlib import pyplot as plt
with plt.ioff():
    fig = plt.figure()
plt.plot([0,1],[0,1])
fig.savefig('test.png')
display(fig)
print(f'Matplotlib version: {matplotlib.__version__}')
print(fig.canvas._is_saving) # no attribute error!!

to give the expected output:
image

I know this isn't the optimal solution but how would you feel about using to fix the immediate problem?

@martinRenou
Copy link
Member

That is very odd, how come the Matplotlib logic fails with the _is_saving trait but is totally fine with other traits 😕.

Isn't the _is_saving property used by the Matplotlib core code?

@tacaswell
Copy link
Member

I would also take out _is_idle_drawing. Both _is_saving and _is_idle_drawing are internal state that Matplotlib uses to communicate with it's self internally. My suspicion is that they just got bulk-up-converted to traitlets during the initial migration.

@tacaswell
Copy link
Member

and if this fixes the problem, then I would say this is the optimal solution as it does not involve core Matplotlib picking up any more complexity and reduces the exposed public surface on the ipympl side.

@tacaswell
Copy link
Member

That is very odd, how come the Matplotlib logic fails with the _is_saving trait but is totally fine with other traits .

Because we are not trying to use other traits in the "set / unset attributes" context manager.

@sjhaque14
Copy link

sjhaque14 commented Oct 19, 2020

I believe I am still having this issue. I am using matplotlib version 3.3.0 and am trying to execute the following code in JupyterLab:

import matplotlib as plt
%matplotlib ipympl
fig = plt.figure()
for i in range(num_simulations):
    plt.plot(mega_affinities[i,:], mega_areas[i,:],label=omega_labels[i])

plt.show()
fig.savefig('greetings')

I get the following error:
Screen Shot 2020-10-19 at 7 39 11 PM

It seems to me as though this error is relevant to the discussion on here, despite this issue being closed. If I am doing something wrong, I do not see what it is.

@scimax
Copy link

scimax commented Oct 20, 2020

For me it was fixed with the new release 0.5.8 after the fix from @ianhi . But I have to admit I moved on to the next matplotlib release already. Are you using ipympl 0.5.8?

Edit: I just saw on the readme page that ipympl 0.5.8 requires matplotlib 3.3.1. Couldn't you move to this one? Or is 3.3.0 necessary?

@sjhaque14
Copy link

That must be it – I can definitely update. Thank you @scimax for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants