Skip to content

BUG: NDFrame.__setattr__ calls object.__setattribute__ on the same name, which can cause unexpected behavior #56794

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

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

Expand Down
62 changes: 28 additions & 34 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
64 changes: 64 additions & 0 deletions pandas/tests/generic/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down