-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
ENH: Allow storing ExtensionArrays in containers #19520
Changes from 138 commits
b7069ef
9cd92c7
9211bbd
80f83a6
ca004d8
5d4a686
9f4ad42
b1db4e8
00d6bb3
1608e3d
52e2180
e6d06e2
d356f19
a6ae340
41f09d8
29cfd7c
05eb9f3
82fd0c6
8b1e7d6
10af4b6
cd5f1eb
0a9d9fd
3185f4e
ffa888a
5a59591
476f75d
b15ee5a
659073f
b15ecac
88b8f4f
349ac1a
27ab045
8358fb1
8ef34a9
340d11b
8297888
7accb67
9b8d2a5
9fbac29
55305dc
0e63708
fbbbc8a
46a0a49
2c4445a
5612cda
b012c19
d49e6aa
d7d31ee
7b89f1b
b0dbffd
66b936f
32ee0ef
a9882e2
f53652a
2425621
fc337de
6abe9da
512fb89
c5db5da
0b112f2
170d0c7
402620f
b556f83
268aabc
d671259
cb740ed
d9e8dd6
815d202
a727b21
f368c29
d74c5c9
0ec6958
8104ee5
4d08218
f8e29b9
0cd9faa
8fcdb70
34a6a22
b4de5c4
c233c28
d6e8051
3af8a21
3a5118b
1e8e87e
0f5e4f0
1436d1d
c4dab88
a312ba5
758689f
02c3d40
9e17037
4599db4
d34d9ca
29d2528
905d72e
412c951
78834f1
f8eac55
f09c863
cedb63d
cae2c26
8088096
8aed325
453728a
cc13c8d
f90ac07
4a03b26
635223f
cf423a7
2d1a66c
c849865
c3ec822
f4cf45c
9c5d479
1175c0d
c816d99
9c9f59e
08af9a3
2e992f7
cc5cc3e
704ee67
c262968
8bf0334
c8d88da
24f3b60
50bd5dd
879bc84
33c9d1f
f07c166
79d43b1
91c629b
a1ebf53
aa57cad
c82748c
e919343
1ea74da
0c41a34
009bece
ea5562b
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 |
---|---|---|
|
@@ -25,14 +25,13 @@ class ExtensionArray(object): | |
* isna | ||
* take | ||
* copy | ||
* _formatting_values | ||
* _concat_same_type | ||
|
||
Some additional methods are required to satisfy pandas' internal, private | ||
Some additional methods are available to satisfy pandas' internal, private | ||
block API. | ||
|
||
* _concat_same_type | ||
* _can_hold_na | ||
* _formatting_values | ||
|
||
This class does not inherit from 'abc.ABCMeta' for performance reasons. | ||
Methods and properties required by the interface raise | ||
|
@@ -53,13 +52,14 @@ class ExtensionArray(object): | |
Extension arrays should be able to be constructed with instances of | ||
the class, i.e. ``ExtensionArray(extension_array)`` should return | ||
an instance, not error. | ||
|
||
Additionally, certain methods and interfaces are required for proper | ||
this array to be properly stored inside a ``DataFrame`` or ``Series``. | ||
""" | ||
# '_typ' is for pandas.core.dtypes.generic.ABCExtensionArray. | ||
# Don't override this. | ||
_typ = 'extension' | ||
# ------------------------------------------------------------------------ | ||
# Must be a Sequence | ||
# ------------------------------------------------------------------------ | ||
|
||
def __getitem__(self, item): | ||
# type (Any) -> Any | ||
"""Select a subset of self. | ||
|
@@ -92,7 +92,46 @@ def __getitem__(self, item): | |
raise AbstractMethodError(self) | ||
|
||
def __setitem__(self, key, value): | ||
# type: (Any, Any) -> None | ||
# type: (Union[int, np.ndarray], Any) -> None | ||
"""Set one or more values inplace. | ||
|
||
This method is not required to satisfy the pandas extension array | ||
interface. | ||
|
||
Parameters | ||
---------- | ||
key : int, ndarray, or slice | ||
When called from, e.g. ``Series.__setitem__``, ``key`` will be | ||
one of | ||
|
||
* scalar int | ||
* ndarray of integers. | ||
* boolean ndarray | ||
* slice object | ||
|
||
value : ExtensionDtype.type, Sequence[ExtensionDtype.type], or object | ||
value or values to be set of ``key``. | ||
|
||
Returns | ||
------- | ||
None | ||
""" | ||
# Some notes to the ExtensionArray implementor who may have ended up | ||
# here. While this method is not required for the interface, if you | ||
# *do* choose to implement __setitem__, then some semantics should be | ||
# observed: | ||
# | ||
# * Setting multiple values : ExtensionArrays should support setting | ||
# multiple values at once, 'key' will be a sequence of integers and | ||
# 'value' will be a same-length sequence. | ||
# | ||
# * Broadcasting : For a sequence 'key' and a scalar 'value', | ||
# each position in 'key' should be set to 'value'. | ||
# | ||
# * Coercion : Most users will expect basic coercion to work. For | ||
# example, a string like '2018-01-01' is coerced to a datetime | ||
# when setting on a datetime64ns array. In general, if the | ||
# __init__ method coerces that value, then so should __setitem__ | ||
raise NotImplementedError(_not_implemented_message.format( | ||
type(self), '__setitem__') | ||
) | ||
|
@@ -107,6 +146,16 @@ def __len__(self): | |
# type: () -> int | ||
raise AbstractMethodError(self) | ||
|
||
def __iter__(self): | ||
"""Iterate over elements of the array. | ||
|
||
""" | ||
# This needs to be implemented so that pandas recognizes extension | ||
# arrays as list-like. The default implementation makes successive | ||
# calls to ``__getitem__``, which may be slower than necessary. | ||
for i in range(len(self)): | ||
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 are you not just raising AbstractMethodError? 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. this is tied intimately with the boxing concept of scalars, but this should be decoupled from getitem (yes its a valid implementation, but you would never actually do it this way), rather you would iterate on some underlying value and box it, not index into and as a side effect use getitem to box. 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.
We don't know how the subclass is storing things though, so there's no 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. so my point is that you cannot implement a default here, make the sub-classes do it. yes it might repeat some code, but its much better there i think. 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.
Yes, we can (see the code). You "don't want" it, that something different :) 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. And some context: If I remove 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 think it still might be better raise here by default and force the subclass to implement this (even if this impl). It is SO important for subclasses to pay attention to this, it should just be required. 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. You can't do much better than this implementation. |
||
yield self[i] | ||
|
||
# ------------------------------------------------------------------------ | ||
# Required attributes | ||
# ------------------------------------------------------------------------ | ||
|
@@ -132,9 +181,9 @@ def nbytes(self): | |
# type: () -> int | ||
"""The number of bytes needed to store this object in memory. | ||
|
||
If this is expensive to compute, return an approximate lower bound | ||
on the number of bytes needed. | ||
""" | ||
# If this is expensive to compute, return an approximate lower bound | ||
# on the number of bytes needed. | ||
raise AbstractMethodError(self) | ||
|
||
# ------------------------------------------------------------------------ | ||
|
@@ -184,8 +233,8 @@ def take(self, indexer, allow_fill=True, fill_value=None): | |
will be done. This short-circuits computation of a mask. Result is | ||
undefined if allow_fill == False and -1 is present in indexer. | ||
fill_value : any, default None | ||
Fill value to replace -1 values with. By default, this uses | ||
the missing value sentinel for this type, ``self._fill_value``. | ||
Fill value to replace -1 values with. If applicable, this should | ||
use the sentinel missing value for this type. | ||
|
||
Notes | ||
----- | ||
|
@@ -198,17 +247,20 @@ def take(self, indexer, allow_fill=True, fill_value=None): | |
|
||
Examples | ||
-------- | ||
Suppose the extension array somehow backed by a NumPy structured array | ||
and that the underlying structured array is stored as ``self.data``. | ||
Then ``take`` may be written as | ||
Suppose the extension array is backed by a NumPy array stored as | ||
``self.data``. Then ``take`` may be written as | ||
|
||
.. code-block:: python | ||
|
||
def take(self, indexer, allow_fill=True, fill_value=None): | ||
mask = indexer == -1 | ||
result = self.data.take(indexer) | ||
result[mask] = self._fill_value | ||
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. hmm, is there a reason we don't define 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. Originally there was a 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. ok that's fine. I think we do need / want a way to set this and a property is good, and |
||
result[mask] = np.nan # NA for this type | ||
return type(self)(result) | ||
|
||
See Also | ||
-------- | ||
numpy.take | ||
""" | ||
raise AbstractMethodError(self) | ||
|
||
|
@@ -230,17 +282,12 @@ def copy(self, deep=False): | |
# ------------------------------------------------------------------------ | ||
# Block-related methods | ||
# ------------------------------------------------------------------------ | ||
@property | ||
def _fill_value(self): | ||
# type: () -> Any | ||
"""The missing value for this type, e.g. np.nan""" | ||
return None | ||
|
||
def _formatting_values(self): | ||
# type: () -> np.ndarray | ||
# At the moment, this has to be an array since we use result.dtype | ||
"""An array of values to be printed in, e.g. the Series repr""" | ||
raise AbstractMethodError(self) | ||
return np.array(self) | ||
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. NO, this should be implemented only by subclasses. 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? 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. This is a very sensible default IMO, subclasses can always override if needed |
||
|
||
@classmethod | ||
def _concat_same_type(cls, to_concat): | ||
|
@@ -257,6 +304,7 @@ def _concat_same_type(cls, to_concat): | |
""" | ||
raise AbstractMethodError(cls) | ||
|
||
@property | ||
def _can_hold_na(self): | ||
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. Interface change: This should have been a property. |
||
# type: () -> bool | ||
"""Whether your array can hold missing values. True by default. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,8 @@ def _check(cls, inst): | |
ABCDateOffset = create_pandas_abc_type("ABCDateOffset", "_typ", | ||
("dateoffset",)) | ||
ABCInterval = create_pandas_abc_type("ABCInterval", "_typ", ("interval", )) | ||
ABCExtensionArray = create_pandas_abc_type("ABCExtensionArray", "_typ", | ||
("extension", "categorical",)) | ||
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. isn't Categorical a first class EA? 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. Yes, but we for Categorical we actually override the |
||
|
||
|
||
class _ABCGeneric(type): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some notes on the semantics of setitem if people want to take a look.