(fix): remove _getattr__ method for PandasExtensionArray
#10250
Conversation
…t resolution, fix code and tests to allow this
for more information, see https://pre-commit.ci
… more carefully, for now using pd.Series to covert `OMm` type datetimes/timedeltas (will result in ns precision)
…rray` series creating an extension array when `.array` is accessed
Co-authored-by: Stephan Hoyer <shoyer@google.com>
|
The core issue seems to be the |
…nto ig/fix_same_reindexer
|
@dcherian We can get rid of it, but then I think we would have to implement everything explicitly. I handled the So the only question is whether we want to implement |
ilan-gold
changed the title
_getattr__ method for PandasExtensionArray
xarray/core/extension_array.py
Outdated
| @@ -109,7 +111,7 @@ def __repr__(self): | |||
| return f"PandasExtensionArray(array={self.array!r})" | |||
|
|
|||
| def __getattr__(self, attr: str) -> object: | |||
| return getattr(self.array, attr) | |||
| return getattr(super().__getattribute__("array"), attr) | |||
There was a problem hiding this comment.
i don't understand this :(. I still lean toward explicit attributes. Subclassing from NdimLenMixin or whatever (see indexing.py) might reduce some pain
There was a problem hiding this comment.
So without __deepcopy__ or __copy__, the object is first constructed and then the sub-objects are attached:
"A shallow copy constructs a new compound object and then (to the extent possible) inserts references into it to the objects found in the original" + "A deep copy constructs a new compound object and then, recursively, inserts copies into it of the objects found in the original."
So without array, this method then calls getattr for "array" again while looking for __setstate__ (which is apparently the first thing sought in copy.copy from the under-construction copied object) because of self.array. __getattribute__ bypasses the lookup mechanism of __getattr__.
See https://stackoverflow.com/questions/40583131/python-deepcopy-with-custom-getattr-and-setattr as well. I will push a commit with all the methods needed enumerated and try to use inheritance
ilan-gold
changed the title
_getattr__ method for PandasExtensionArray_getattr__ method for PandasExtensionArray
|
@dcherian Removing the method revealed some other small things I cleaned up including a bug in |
xarray/core/extension_array.py
Outdated
| return item | ||
| return PandasExtensionArray(item) | ||
| if np.isscalar(item) or isinstance(key, int): | ||
| return PandasExtensionArray(type(self.array)._from_sequence([item])) # type: ignore[call-arg] # only subclasses with proper __init__ allowed |
There was a problem hiding this comment.
do we need the _from_sequence, looks sketchy in the long-term
There was a problem hiding this comment.
agreed, not sure why these underscore methods are in the public api, but they appear to be https://pandas.pydata.org/docs/reference/api/pandas.api.extensions.ExtensionArray._from_sequence.html so that's what im going on
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
This PR makes explicit the methods needed for duck array typing instead of using
getattron the underlying array (thereby resolving the linked issue). It also fixes a bug in__getitem__which should further help solidify the API (which was typed correctly).whats-new.rstapi.rst