From 2706fa6d84df3dd848099b2e2b962a4285bf6374 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 16 Jun 2020 15:00:28 -0400 Subject: [PATCH 1/2] MNT: make _setattr_cm more forgiving 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 #17646 --- lib/matplotlib/cbook/__init__.py | 22 +++++++++++++++------- lib/matplotlib/tests/test_cbook.py | 22 +++++++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/matplotlib/cbook/__init__.py b/lib/matplotlib/cbook/__init__.py index 05631ef4ca45..77af716147a9 100644 --- a/lib/matplotlib/cbook/__init__.py +++ b/lib/matplotlib/cbook/__init__.py @@ -2035,20 +2035,28 @@ def _setattr_cm(obj, **kwargs): origs = {} for attr in kwargs: 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 an general descriptor) + # we want to set the original value back. if isinstance(cls_orig, property): origs[attr] = orig - elif isinstance(cls_orig, types.FunctionType): - origs[attr] = sentinel + # 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 an 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: - raise ValueError( - f"trying to set {attr} which is not a method, " - "property, or instance level attribute" - ) + origs[attr] = sentinel try: for attr, val in kwargs.items(): diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index 4e4457e2a392..f6429cf9ecc5 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -666,6 +666,14 @@ def __init__(self): def meth(self): ... + @classmethod + def classy(klass): + ... + + @staticmethod + def static(): + ... + @property def prop(self): return self._p @@ -696,6 +704,10 @@ def verify_pre_post_state(obj): assert not hasattr(obj, 'extra') assert obj.prop == 'p' assert obj.monkey == other.meth + assert obj.cls_level is A.cls_level + assert 'cls_level' not in obj.__dict__ + assert 'classy' not in obj.__dict__ + assert 'static' not in obj.__dict__ a = B() @@ -705,7 +717,8 @@ def verify_pre_post_state(obj): a, prop='squirrel', aardvark='moose', meth=lambda: None, override='boo', extra='extra', - monkey=lambda: None): + monkey=lambda: None, cls_level='bob', + classy='classy', static='static'): # because we have set a lambda, it is normal attribute access # and the same every time assert a.meth is a.meth @@ -715,9 +728,8 @@ def verify_pre_post_state(obj): assert a.extra == 'extra' assert a.prop == 'squirrel' assert a.monkey != other.meth + assert a.cls_level == 'bob' + assert a.classy == 'classy' + assert a.static == 'static' verify_pre_post_state(a) - - with pytest.raises(ValueError): - with cbook._setattr_cm(a, cls_level='bob'): - pass From e11316a849359f551106fcdec89e17ec639207ba Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 16 Jun 2020 17:55:06 -0400 Subject: [PATCH 2/2] DOC: correct grammar in comments Co-authored-by: Elliott Sales de Andrade --- lib/matplotlib/cbook/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/cbook/__init__.py b/lib/matplotlib/cbook/__init__.py index 77af716147a9..873d46c9997c 100644 --- a/lib/matplotlib/cbook/__init__.py +++ b/lib/matplotlib/cbook/__init__.py @@ -2043,7 +2043,7 @@ def _setattr_cm(obj, **kwargs): # 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 an general descriptor) + # 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 @@ -2051,7 +2051,7 @@ def _setattr_cm(obj, **kwargs): # 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 an non-property custom descriptor which + # 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.