diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst index d9ab0452c8334..fcd52c33be8d7 100644 --- a/doc/source/whatsnew/v2.2.0.rst +++ b/doc/source/whatsnew/v2.2.0.rst @@ -845,6 +845,7 @@ Indexing - Bug in :meth:`DataFrame.loc` mutating a boolean indexer when :class:`DataFrame` has a :class:`MultiIndex` (:issue:`56635`) - Bug in :meth:`DataFrame.loc` when setting :class:`Series` with extension dtype into NumPy dtype (:issue:`55604`) - Bug in :meth:`Index.difference` not returning a unique set of values when ``other`` is empty or ``other`` is considered non-comparable (:issue:`55113`) +- Bug in :meth:`NDFrame.__setattr__` getting from a property before setting to it if extending :class:`DataFrame` with a property (:issue:`56773`) - Bug in setting :class:`Categorical` values into a :class:`DataFrame` with numpy dtypes raising ``RecursionError`` (:issue:`52927`) - Fixed bug when creating new column with missing values when setting a single string value (:issue:`56204`) diff --git a/pandas/core/generic.py b/pandas/core/generic.py index 73f9481e53dea..c080341f39c32 100644 --- a/pandas/core/generic.py +++ b/pandas/core/generic.py @@ -6291,44 +6291,38 @@ def __getattr__(self, name: str): @final def __setattr__(self, name: str, value) -> None: """ - After regular attribute access, try setting the name + If it can be found nowhere else than the info_axis, it must be a column. This allows simpler access to columns for interactive use. """ - # first try regular attribute access via __getattribute__, so that - # e.g. ``obj.x`` and ``obj.x = 4`` will always reference/modify - # the same attribute. - - try: - object.__getattribute__(self, name) - return object.__setattr__(self, name, value) - except AttributeError: - pass - - # if this fails, go on to more involved attribute setting - # (note that this matches __getattr__, above). - if name in self._internal_names_set: - object.__setattr__(self, name, value) - elif name in self._metadata: - object.__setattr__(self, name, value) - else: + if ( + name not in self.__dict__ + and name not in self._internal_names_set + and name not in self._metadata + and name in self._info_axis + ): try: - existing = getattr(self, name) - if isinstance(existing, Index): - object.__setattr__(self, name, value) - elif name in self._info_axis: - self[name] = value - else: - object.__setattr__(self, name, value) + self[name] = value except (AttributeError, TypeError): - if isinstance(self, ABCDataFrame) and (is_list_like(value)): - warnings.warn( - "Pandas doesn't allow columns to be " - "created via a new attribute name - see " - "https://pandas.pydata.org/pandas-docs/" - "stable/indexing.html#attribute-access", - stacklevel=find_stack_level(), - ) - object.__setattr__(self, name, value) + pass + else: + return + + if ( + isinstance(self, ABCDataFrame) + and name not in self.__dict__ + and not hasattr(type(self), name) + and name not in self._internal_names + and name not in self._metadata + and is_list_like(value) + ): + warnings.warn( + "Pandas doesn't allow columns to be " + "created via a new attribute name - see " + "https://pandas.pydata.org/pandas-docs/" + "stable/indexing.html#attribute-access", + stacklevel=find_stack_level(), + ) + object.__setattr__(self, name, value) @final def _dir_additions(self) -> set[str]: diff --git a/pandas/tests/generic/test_frame.py b/pandas/tests/generic/test_frame.py index fc7aa9e7b2c46..1380b7fe3a53d 100644 --- a/pandas/tests/generic/test_frame.py +++ b/pandas/tests/generic/test_frame.py @@ -149,6 +149,70 @@ def test_set_attribute(self): assert df.y == 5 tm.assert_series_equal(df["y"], Series([2, 4, 6], name="y")) + def test_setattr_doesnt_call_getattr(self): + # Test to assert setattr doesn't call getattr, resulting in potentially + # odd behavior for descriptors; this is a step towards pandas extensibility + + from functools import cached_property + + class FancyDescriptor: + def __get__(self, instance, owner): + if instance is None: + return self + return instance.__dict__[self.__name__] + + def __set__(self, instance, value): + instance.__dict__[self.__name__] = value + + def __delete__(self, instance): + del instance.__dict__[self.__name__] + + def __set_name__(self, owner, name): + self.__name__ = name + + class Frame(DataFrame): + @cached_property + def scalar(self): + raise AttributeError + + @cached_property + def vector(self): + raise AttributeError + + @property + def scalar_(self): + return self.__dict__["scalar_"] + + @scalar_.setter + def scalar_(self, value): + self.__dict__["scalar_"] = value + + @property + def vector_(self): + return self.__dict__["vector_"] + + @vector_.setter + def vector_(self, value): + self.__dict__["vector_"] = value + + fancy = FancyDescriptor() + + with tm.assert_produces_warning(False): + frame = Frame() + try: + frame.scalar = ... + frame.vector = [1, 2, 3] + frame.scalar_ = ... + frame.vector_ = [1, 2, 3] + frame.fancy = ... + del frame.fancy + frame.fancy = [1, 2, 3] + except (AttributeError, KeyError): + pytest.fail( + "DataFrame.__setattr__ prematurely called " + "getattr on the same name." + ) + def test_deepcopy_empty(self): # This test covers empty frame copying with non-empty column sets # as reported in issue GH15370