-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
Array Interface and Categorical internals Refactor #19268
Changes from 21 commits
2ef5216
57e8b0f
01bd42f
ce81706
87a70e3
8c61886
cb41803
65d5a61
57c749b
6736b0f
e4acb59
0e9337b
df68f3b
2746a43
34d2b99
a484d61
04b2e72
df0fa12
e778053
d15a722
b5f736d
240e8f6
f9b0b49
7913186
df18c3b
ab2f045
520876f
4dfa39c
e252103
7110b2a
c59dca0
fc688a5
fbc8466
030bb19
0f4c2d7
f9316e0
9c06b13
7d2cf9c
afae8ae
cd0997e
1d6eb04
92aed49
34134f2
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 |
---|---|---|
@@ -1 +1,2 @@ | ||
from .base import ExtensionArray # noqa | ||
from .categorical import Categorical # noqa |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,191 @@ | ||
"""An interface for extending pandas with custom arrays.""" | ||
import abc | ||
|
||
import numpy as np | ||
|
||
from pandas.compat import add_metaclass | ||
|
||
|
||
_not_implemented_message = "{} does not implement {}." | ||
|
||
|
||
@add_metaclass(abc.ABCMeta) | ||
class ExtensionArray(object): | ||
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. Are there any expected requirements for the constructor 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. Yeah, we should figure out what those are and document them. At the very least, we expected 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 also expect that Rather than imposing that on subclasses, we could require some kind of |
||
"""Abstract base class for custom array types | ||
|
||
Notes | ||
----- | ||
pandas will recognize instances of this class as proper arrays | ||
with a custom type and will not attempt to coerce them to objects. | ||
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. needs much more detail about what is expected here (or at least some examples), e.g. 1-D array-like or whatever 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 we can leave general docs until this is actually working (the above sentence is at the moment not yet true), which will only be in follow-up PRs 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. Added a bit about 1-D and some high-level examples. |
||
|
||
**Restrictions on your class constructor** | ||
|
||
* Extension arrays should be able to be constructed with instances of | ||
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 formatting is a bit odd here |
||
the class, i.e. ``ExtensionArray(extension_array)`` should return | ||
an instance, not error. | ||
""" | ||
# ------------------------------------------------------------------------ | ||
# Must be a Sequence | ||
# ------------------------------------------------------------------------ | ||
@abc.abstractmethod | ||
def __getitem__(self, item): | ||
# type (Any) -> Any | ||
"""Select a subset of 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. add Parameters / Returns |
||
|
||
Notes | ||
----- | ||
``item`` may be one of | ||
|
||
* A scalar integer position | ||
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. part of Parameters |
||
* A slice object, where 'start', 'stop', and 'step' are | ||
integers or None | ||
* A 1-d boolean NumPy ndarray the same length as '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. this actually just needs to be convertible to a numpy array 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 should be strict about how we use this interface from pandas. I see little harm in saying this is always a NumPy array since we are the only ones calling it and some clear advantages. Otherwise, how is a user supposed to detect this case? |
||
|
||
For scalar ``item``, return a scalar value suitable for the array's | ||
type. This should be an instance of ``self.dtype.type``. | ||
|
||
For slice ``key``, return an instance of ``ExtensionArray``, even | ||
if the slice is length 0 or 1. | ||
|
||
For a boolean mask, return an instance of ``ExtensionArray``, filtered | ||
to the values where ``item`` is True. | ||
""" | ||
|
||
def __setitem__(self, key, value): | ||
# type: (Any, Any) -> None | ||
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 already use AbstractMethodError elsewhere in the code base, use instead 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. setitem must be defined (it certainly does not need to actually set inplace), but since we use this is must appear as a mutable object 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. The point of having it here as NotImplementedError is because an ExtensionArray does not necessarily needs to support setting elements (being mutable), at least that's one possible decision (leave the decision up to the extension author). 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. Yep, I don't think we can assume that all extension arrays will implement it. |
||
raise NotImplementedError(_not_implemented_message.format( | ||
type(self), '__setitem__') | ||
) | ||
|
||
@abc.abstractmethod | ||
def __len__(self): | ||
# type: () -> int | ||
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. document |
||
pass | ||
|
||
# ------------------------------------------------------------------------ | ||
# Required attributes | ||
# ------------------------------------------------------------------------ | ||
@property | ||
@abc.abstractmethod | ||
def dtype(self): | ||
# type: () -> ExtensionDtype | ||
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. examples and document 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. What would an example even look like here? Defining an |
||
"""An instance of 'ExtensionDtype'.""" | ||
|
||
@property | ||
def shape(self): | ||
# type: () -> Tuple[int, ...] | ||
return (len(self),) | ||
|
||
@property | ||
def ndim(self): | ||
# type: () -> int | ||
"""Extension Arrays are only allowed to be 1-dimensional.""" | ||
return 1 | ||
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 should be tested on registration of the sub-type 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. What do you mean "registration"? We could override If people want to mess with this, that's fine, their stuff just won't work with pandas. 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. what I mean is that when you register things, we should actually test that the interface is respected. If we had final methods this would not be necessary, but if someone override ndim this is a problem. |
||
|
||
@property | ||
@abc.abstractmethod | ||
def nbytes(self): | ||
# type: () -> int | ||
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. Type comments come before the docstring: http://mypy.readthedocs.io/en/latest/python2.html |
||
"""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. | ||
""" | ||
|
||
# ------------------------------------------------------------------------ | ||
# Additional Methods | ||
# ------------------------------------------------------------------------ | ||
@abc.abstractmethod | ||
def isna(self): | ||
# type: () -> np.ndarray | ||
"""Boolean NumPy array indicating if each value is missing.""" | ||
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. same length as self |
||
|
||
# ------------------------------------------------------------------------ | ||
# Indexing methods | ||
# ------------------------------------------------------------------------ | ||
@abc.abstractmethod | ||
def take(self, indexer, allow_fill=True, fill_value=None): | ||
# type: (Sequence[int], bool, Optional[Any]) -> ExtensionArray | ||
"""Take elements from an array. | ||
|
||
Parameters | ||
---------- | ||
indexer : sequence of integers | ||
indices to be taken. -1 is used to indicate values | ||
that are missing. | ||
allow_fill : bool, default True | ||
If False, indexer is assumed to contain no -1 values so no filling | ||
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``. | ||
|
||
Notes | ||
----- | ||
This should follow pandas' semantics where -1 indicates missing values. | ||
Positions where indexer is ``-1`` should be filled with the missing | ||
value for this type. | ||
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. Let's strongly consider exposing a helper function to make this easier to write, or at least an example of what this would look like (we can save this for later). It's not obvious how to write this 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. I added an example that I hope gets things correct for an extension type backed by a NumPy structured array. One trouble with this providing a helper function is that we don't know much about how the extension array is actually storing the data. Although, we could rely on the assumption that the underlying storage is convertible to a NumPy array, and proceed from there. Though this would perhaps be sub-optimal for many extension arrays. 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. For GeometryArray, we have followed the same idea the write 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 fine as it is a pandas standard. |
||
|
||
This is called by ``Series.__getitem__``, ``.loc``, ``iloc``, when the | ||
indexer is a sequence of values. | ||
|
||
Examples | ||
-------- | ||
Suppose the extension array somehow backed by a NumPy structured array | ||
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. let's make this just "NumPy array", as this is not specific to structured arrays |
||
and that the underlying structured array is 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. One question here is: should the keyword argument In any case some clarification would be helpful, also if it is just in the signature for compatibility but may be ignored (maybe in a follow-up). 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. re I think that since ExtensionArray.take returns an ExtensionArray, most implementations will just ignore fill_value. I'll clarify it in the docs. |
||
return type(self)(result) | ||
""" | ||
|
||
@abc.abstractmethod | ||
def copy(self, deep=False): | ||
# type: (bool) -> ExtensionArray | ||
"""Return a copy of the array.""" | ||
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. document. |
||
|
||
# ------------------------------------------------------------------------ | ||
# Block-related methods | ||
# ------------------------------------------------------------------------ | ||
@property | ||
def _fill_value(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. need to list this in the very top doc-string |
||
# type: () -> Any | ||
"""The missing value for this type, e.g. np.nan""" | ||
return None | ||
|
||
@abc.abstractmethod | ||
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""" | ||
|
||
@classmethod | ||
@abc.abstractmethod | ||
def _concat_same_type(cls, to_concat): | ||
# type: (Sequence[ExtensionArray]) -> ExtensionArray | ||
"""Concatenate multiple array | ||
|
||
Parameters | ||
---------- | ||
to_concat : sequence of this type | ||
|
||
Returns | ||
------- | ||
ExtensionArray | ||
""" | ||
|
||
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. need to list this in the very top doc-string |
||
# type: () -> bool | ||
"""Whether your array can hold missing values. True by default. | ||
|
||
Notes | ||
----- | ||
Setting this to false will optimize some operations like fillna. | ||
""" | ||
return True |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,8 @@ | |
from pandas.util._validators import validate_bool_kwarg | ||
from pandas.core.config import get_option | ||
|
||
from .base import ExtensionArray | ||
|
||
|
||
def _cat_compare_op(op): | ||
def f(self, other): | ||
|
@@ -148,7 +150,7 @@ def _maybe_to_categorical(array): | |
""" | ||
|
||
|
||
class Categorical(PandasObject): | ||
class Categorical(ExtensionArray, PandasObject): | ||
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. By having our internal arrays inherit from 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. yeah, the methods in PandasObject needs to be ABC in the ExtensionArray 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. On the other hand, I don't think all methods/attributes of PandasObject should be added to the public ExtensionArray (to keep those internal + to not clutter the ExtensionArray API) 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. FYI, I'm consistently testing these changes against
Neither inherit from |
||
""" | ||
Represents a categorical variable in classic R / S-plus fashion | ||
|
||
|
@@ -2130,6 +2132,21 @@ def repeat(self, repeats, *args, **kwargs): | |
return self._constructor(values=codes, categories=self.categories, | ||
ordered=self.ordered, fastpath=True) | ||
|
||
# Interface things | ||
# can_hold_na, concat_same_type, formatting_values | ||
@property | ||
def _can_hold_na(self): | ||
return True | ||
|
||
@classmethod | ||
def _concat_same_type(self, to_concat): | ||
from pandas.core.dtypes.concat import _concat_categorical | ||
|
||
return _concat_categorical(to_concat) | ||
|
||
def _formatting_values(self): | ||
return self | ||
|
||
# The Series.cat accessor | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
"""Extend pandas with custom array types""" | ||
import abc | ||
|
||
from pandas.compat import add_metaclass | ||
|
||
|
||
@add_metaclass(abc.ABCMeta) | ||
class ExtensionDtype(object): | ||
"""A custom data type for your array. | ||
""" | ||
|
||
def __str__(self): | ||
return self.name | ||
|
||
@property | ||
@abc.abstractmethod | ||
def type(self): | ||
# type: () -> type | ||
"""The scalar type for your array, e.g. ``int`` | ||
|
||
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. document more on these 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. can you be more clear in what you expect more? The sentence below here seems rather explicit to me. |
||
It's expected ``ExtensionArray[item]`` returns an instance | ||
of ``ExtensionDtype.type`` for scalar ``item``. | ||
""" | ||
|
||
@property | ||
def kind(self): | ||
# type () -> str | ||
"""A character code (one of 'biufcmMOSUV'), default 'O' | ||
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 should clarify how it's used. How is this useful? Perhaps "This should match |
||
|
||
This should match the NumPy dtype used when your array is | ||
converted to an ndarray, which is probably 'O' for object if | ||
your extension type cannot be represented as a built-in NumPy | ||
type. | ||
|
||
See Also | ||
-------- | ||
numpy.dtype.kind | ||
""" | ||
return 'O' | ||
|
||
@property | ||
@abc.abstractmethod | ||
def name(self): | ||
# type: () -> str | ||
"""A string identifying the data type. | ||
|
||
Will be used for display in, e.g. ``Series.dtype`` | ||
""" | ||
|
||
@property | ||
def names(self): | ||
# type: () -> Optional[List[str]] | ||
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. what is this for? 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. Numpy has this for structured dtypes. Not sure if this is needed in the interface though (I don't think we will use it internally, on the other hand this is compatible 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. We use it in elif isinstance(data, (np.ndarray, Series, Index)):
if data.dtype.names: I could of course modify that, but perhaps as a followup. For now having a non-abstract names property seems OK. 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. add an example here (or maybe more expl) |
||
"""Ordered list of field names, or None if there are no fields.""" | ||
return None | ||
|
||
@classmethod | ||
@abc.abstractmethod | ||
def construct_from_string(cls, string): | ||
"""Attempt to construct this type from a string. | ||
|
||
Parameters | ||
---------- | ||
string : str | ||
|
||
Returns | ||
------- | ||
self : instance of 'cls' | ||
|
||
Raises | ||
------ | ||
TypeError | ||
If a class cannot be constructed from this 'string'. | ||
|
||
Notes | ||
----- | ||
The default implementation checks if 'string' matches your | ||
type's name. If so, it calls your class with no arguments. | ||
|
||
Examples | ||
-------- | ||
If the extension dtype can be constructed without any arguments, | ||
the following may be an adequate implementation. | ||
|
||
>>> @classmethod | ||
... def construct_from_string(cls, string) | ||
... if string == cls.name: | ||
... return cls() | ||
... else: | ||
... raise TypeError("Cannot construct a '{}' from " | ||
... "'{}'".format(cls, string)) | ||
""" | ||
|
||
@classmethod | ||
def is_dtype(cls, dtype): | ||
"""Check if we match 'dtype' | ||
|
||
Parameters | ||
---------- | ||
dtype : str or dtype | ||
|
||
Returns | ||
------- | ||
is_dtype : bool | ||
|
||
Notes | ||
----- | ||
The default implementation is True if | ||
|
||
1. ``cls.construct_from_string(dtype)`` is an instance | ||
of ``cls``. | ||
2. 'dtype' is ``cls`` or a subclass of ``cls``. | ||
""" | ||
if isinstance(dtype, str): | ||
try: | ||
return isinstance(cls.construct_from_string(dtype), cls) | ||
except TypeError: | ||
return False | ||
else: | ||
return issubclass(dtype, cls) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1685,6 +1685,38 @@ def is_extension_type(arr): | |
return False | ||
|
||
|
||
def is_extension_array_dtype(arr_or_dtype): | ||
"""Check if an object is a pandas extension array type | ||
|
||
Parameters | ||
---------- | ||
arr_or_dtype : object | ||
|
||
Returns | ||
------- | ||
bool | ||
|
||
Notes | ||
----- | ||
This checks whether an object implements the pandas extension | ||
array interface. In pandas, this includes: | ||
|
||
* Categorical | ||
* PeriodArray | ||
* IntervalArray | ||
* SparseArray | ||
|
||
Third-party libraries may implement arrays or types satisfying | ||
this interface as well. | ||
""" | ||
from pandas.core.arrays import ExtensionArray | ||
|
||
# we want to unpack series, anything else? | ||
if isinstance(arr_or_dtype, ABCSeries): | ||
arr_or_dtype = arr_or_dtype._values | ||
return isinstance(arr_or_dtype, (ExtensionDtype, ExtensionArray)) | ||
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 need to call 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. If you pass this function an ExtensionArray subclass, you will get that here? 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.
The result is just True or False. The argument can be either an array or dtype. I'm not sure that 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 not following what we do in all other cases that's my point. pls use _get_dtype_or_type 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.
That won't work unfortunately. In [1]: import pandas as pd; import pandas_ip as ip
In [2]: arr = ip.IPAddress([1, 2, 3])
In [3]: pd.core.dtypes.common._get_dtype_type(arr)
Out[3]: pandas_ip.block.IPv4v6Base
In [4]: isinstance(arr[0], ip.block.IPv4v6Base)
Out[4]: True
In [5]: issubclass(ip.block.IPv4v6Base, pd.core.dtypes.base.ExtensionDtype)
Out[5]: 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. then 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. But that would make 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.
What's the issue with the function as defined? I need a way to tell if an array or dtype is an ExtensionArray or ExtensionDtype. Someday, when |
||
|
||
|
||
def is_complex_dtype(arr_or_dtype): | ||
""" | ||
Check whether the provided array or dtype is of a complex dtype. | ||
|
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.
I'd advocate leaving base.py open for (near-)future usage as pandas-internal base and putting the "use this if you want to write your own" file in e.g. extension.py
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.
I have a slight preference for
base.py
since it's a base class for all extension arrays. I don't think that havingExtensionArray
inarrays.base
precludes having a pandas-internal base there as well.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.
It will be publicly exposed through pd.api.extensions anyway I think
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.
Adding stuff to the public API is waiting on #19304