-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
CoW: add readonly flag to ExtensionArrays, return read-only EA/ndarray in .array/EA.to_numpy() #61925
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
base: main
Are you sure you want to change the base?
CoW: add readonly flag to ExtensionArrays, return read-only EA/ndarray in .array/EA.to_numpy() #61925
Changes from all commits
a9df51b
9cd6e4f
c6f37d1
8058d9a
91465ee
856dc02
828fadc
ee1ed6e
3f7bc3e
1765fe7
a7abee3
4b6ced0
f235aa3
f76bbc8
5cfb0f8
ab4b976
6e08cc8
5a5170e
af62da8
0af4b39
959451e
a4accf8
d5d4db4
84e83c7
6ec9b06
9f78d76
f6f300e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,7 @@ | |
validate_insert_loc, | ||
) | ||
|
||
from pandas.core.dtypes.astype import astype_is_view | ||
from pandas.core.dtypes.common import ( | ||
is_list_like, | ||
is_scalar, | ||
|
@@ -266,6 +267,8 @@ class ExtensionArray: | |
# strictly less than 2000 to be below Index.__pandas_priority__. | ||
__pandas_priority__ = 1000 | ||
|
||
_readonly = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use arr.flags.writeable to be consistent with numpy? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this was easier for a quick POC ;) |
||
|
||
# ------------------------------------------------------------------------ | ||
# Constructors | ||
# ------------------------------------------------------------------------ | ||
|
@@ -428,6 +431,22 @@ def __getitem__(self, item: PositionalIndexer) -> Self | Any: | |
""" | ||
raise AbstractMethodError(self) | ||
|
||
def _getitem_returns_view(self, key) -> bool: | ||
if not isinstance(key, tuple): | ||
key = (key,) | ||
|
||
# filter out Ellipsis and np.newaxis | ||
key = tuple(k for k in key if k is not Ellipsis and k is not np.newaxis) | ||
if not key: | ||
return True | ||
# single integer gives view if selecting subset of 2D array | ||
if self.ndim == 2 and lib.is_integer(key[0]): | ||
return True | ||
# slices always give views | ||
if all(isinstance(k, slice) for k in key): | ||
return True | ||
return False | ||
|
||
def __setitem__(self, key, value) -> None: | ||
""" | ||
Set one or more values inplace. | ||
|
@@ -452,6 +471,11 @@ def __setitem__(self, key, value) -> None: | |
Returns | ||
------- | ||
None | ||
|
||
Raises | ||
------ | ||
ValueError | ||
If the array is readonly and modification is attempted. | ||
""" | ||
# Some notes to the ExtensionArray implementer who may have ended up | ||
# here. While this method is not required for the interface, if you | ||
|
@@ -471,6 +495,10 @@ def __setitem__(self, key, value) -> None: | |
# __init__ method coerces that value, then so should __setitem__ | ||
# Note, also, that Series/DataFrame.where internally use __setitem__ | ||
# on a copy of the data. | ||
# Check if the array is readonly | ||
if self._readonly: | ||
raise ValueError("Cannot modify read-only array") | ||
|
||
raise NotImplementedError(f"{type(self)} does not implement __setitem__.") | ||
|
||
def __len__(self) -> int: | ||
|
@@ -565,8 +593,14 @@ def to_numpy( | |
result = np.asarray(self, dtype=dtype) | ||
if copy or na_value is not lib.no_default: | ||
result = result.copy() | ||
elif self._readonly and astype_is_view(self.dtype, result.dtype): | ||
# If the ExtensionArray is readonly, make the numpy array readonly too | ||
result = result.view() | ||
result.flags.writeable = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be done below the setting of na_value on L616? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so, because in that case the |
||
|
||
if na_value is not lib.no_default: | ||
result[self.isna()] = na_value # type: ignore[index] | ||
|
||
return result | ||
|
||
# ------------------------------------------------------------------------ | ||
|
Uh oh!
There was an error while loading. Please reload this page.