REF: move MaskedArray subclass attributes to dtypes #58423
Conversation
pandas/core/dtypes/dtypes.py
Outdated
| _internal_fill_value: Scalar | ||
|
|
||
| @property | ||
| def truthy_value(self): |
There was a problem hiding this comment.
Should these be "private" ie `_truthy_value?
There was a problem hiding this comment.
they are accessed from outside the dtype's methods
There was a problem hiding this comment.
Shouldn't these raise NotImplementedError as a fallback? The idea is this class can be used for more than just float/integer/bool in the future right?
There was a problem hiding this comment.
i dont think there's any plans to extend like that
There was a problem hiding this comment.
On second thought decided to privatize after all
pandas/core/dtypes/dtypes.py
Outdated
| _internal_fill_value: Scalar | ||
|
|
||
| @property | ||
| def truthy_value(self): |
There was a problem hiding this comment.
Shouldn't these raise NotImplementedError as a fallback? The idea is this class can be used for more than just float/integer/bool in the future right?
| values = np.empty(shape, dtype=dtype.type) | ||
| values.fill(cls._internal_fill_value) | ||
| dtype = cast(BaseMaskedDtype, dtype) | ||
| values: np.ndarray = np.empty(shape, dtype=dtype.type) |
There was a problem hiding this comment.
Is the type declaration required here? I assume mypy would be able to infer this from the right hand side
There was a problem hiding this comment.
mypy complained without this
| @@ -155,8 +148,9 @@ def _from_sequence(cls, scalars, *, dtype=None, copy: bool = False) -> Self: | |||
| @classmethod | |||
| @doc(ExtensionArray._empty) | |||
| def _empty(cls, shape: Shape, dtype: ExtensionDtype) -> Self: | |||
| values = np.empty(shape, dtype=dtype.type) | |||
| values.fill(cls._internal_fill_value) | |||
| dtype = cast(BaseMaskedDtype, dtype) | |||
There was a problem hiding this comment.
The cast suspicious - why wouldn't we just change the type of dtype to BaseMaskedType if the _internal_fill_value attribute was required?
There was a problem hiding this comment.
IIRC mypy complains if we do that, but in practice that is correct that we should only ever get BaseMaskedDtype here
mroeschke
added
the
NA - MaskedArrays
|
Thanks @jbrockmendel |
Discussed on yesterday's dev call, incremental steps towards merging IntegerArray/FloatingArray/BooleanArray into a single class