Skip to content

more conservative setattr_cm broke mplcairo #17646

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.

Sign up for GitHub

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

Jump to bottom
Closed
anntzer opened this issue Jun 16, 2020 · 1 comment · Fixed by #17649
Closed

more conservative setattr_cm broke mplcairo #17646

anntzer opened this issue Jun 16, 2020 · 1 comment · Fixed by #17649
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone
v3.3.0

Comments

@anntzer
Copy link
Contributor

anntzer commented Jun 16, 2020

Trying to run tight_layout with mplcairo broke since #17620 with

ValueError: trying to set draw_gouraud_triangles which is not a method, property, or instance level attribute

because the check isinstance(cls_orig, types.FunctionType) fails for extension methods (such as GraphicsContextRendererCairo.<any method>). Looks like a better check is actually something like isinstance(orig, types.MethodType) and hasattr(type(obj), attr) (the first isinstance check verifies that orig is a bound method and then we just check that the attribute exists on the class and assume that if invoking the descriptor on it gives us a bound method, then we're good) -- at least it works for mplcairo...
@anntzer anntzer added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Jun 16, 2020
@anntzer anntzer added this to the v3.3.0 milestone Jun 16, 2020
@anntzer anntzer mentioned this issue Jun 16, 2020
Closed
6 tasks
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Jun 16, 2020
@tacaswell
MNT: make _setattr_cm more forgiving
2706fa6 
Our attempts to identify the set of cases we wanted to support did not
correctly capture all relevant cases.  This tries to simplify the
logic:

 - if the attribute is in the instance dict, stash and restore it via
   setattr at the end
 - if the attribute is not on the object, delete it with delattr at
   the end
 - if the object has the attribute, but it is not in the instance
   dict:
   - if it is a property, stash and restore the old value
   - in all other cases assume that setattr will put the value in the
     instance dict and delattr will do what we want on the way out

closes matplotlib#17646
@tacaswell tacaswell mentioned this issue Jun 16, 2020
Merged
2 tasks
@tacaswell tacaswell modified the milestones: v3.3.0, v3.2.2 Jun 16, 2020
@tacaswell
Copy link
Member

I thought we had backported #17620

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.3.0
2 participants
@tacaswell @anntzer