Convert ExtensionDtype class objects to instances of that class #47108
Conversation
And parameterize while we're at it.
|
Ahh, thanks for the clarification. I must have misread #45417 (comment) to mean that the changes here were desirable. Just to confirm (please correct as needed):
|
|
yep that sounds right |
This just rearranges lines, for easier visual grepping.
patrickmckenna
force-pushed
the
dtype-handling
branch
from
22d6d50 to
7d331f4
Compare
| @@ -67,6 +68,12 @@ | |||
| _is_scipy_sparse = None | |||
|
|
|||
| ensure_float64 = algos.ensure_float64 | |||
| ensure_int64 = algos.ensure_int64 | |||
There was a problem hiding this comment.
The changed line count in this module look gnarly, but theres's only one functional change (pointed out below). Everything is else is just rearranging lines so that like functions are grouped together, for easier visual grepping and maintenance.
For example, this module defines 5 is_datetime*_dtype(arr_or_dtype) functions. They used be in 2 different sections 500 lines apart, but are now grouped together.
| @@ -1760,6 +1599,10 @@ def pandas_dtype(dtype) -> DtypeObj: | |||
| ------ | |||
| TypeError if not a dtype | |||
| """ | |||
| if inspect.isclass(dtype) and issubclass(dtype, (np.dtype, ExtensionDtype)): | |||
There was a problem hiding this comment.
The one substantive change in this module is these 3 lines.
| @@ -24,6 +29,28 @@ | |||
| from pandas.arrays import SparseArray | |||
|
|
|||
|
|
|||
| @pytest.fixture(name="ea_dtype", params=_registry.dtypes, scope="module") | |||
There was a problem hiding this comment.
This seemed like the most efficient way to retrieve all ExtensionDtype subclasses, but please LMK if there's a different, preferred method. I didn't see any existing fixture, or constant in _test/__init__.py, that enumerated these.
| params=(f for f in dir(com) if re.fullmatch(r"^is_\w+_dtype$", f)), | ||
| scope="module", | ||
| ) | ||
| def fixture_is_dtype_func(request): |
There was a problem hiding this comment.
Move a pre-existing function (get_is_dtype_funcs) into a fixture, since its output is now used by multiple tests.
| @@ -107,6 +147,37 @@ def test_period_dtype(self, dtype): | |||
| assert com.pandas_dtype(dtype) == dtype | |||
|
|
|||
|
|
|||
| @pytest.mark.xfail(reason="not yet implemented") | |||
There was a problem hiding this comment.
This test currently fails for all parameterizations.
It sounds like the is_*_dtype functions should be changed so that this tests passes, but I left that out of this PR for 2 reasons:
-
Modifying only
pandas_dtypeis enough to get a (hopefully) more useful error message, which was the request in AssertionError creating array with StringDtype instead of StringDtype(), can we have a friendlier error message? #31356.>>> pd.Series(dtype=pd.Int64Dtype) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/pandas/pandas/core/series.py", line 395, in __init__ dtype = self._validate_dtype(dtype) File "/home/pandas/pandas/core/generic.py", line 436, in _validate_dtype dtype = pandas_dtype(dtype) File "/home/pandas/pandas/core/dtypes/common.py", line 1604, in pandas_dtype raise TypeError(msg) TypeError: Must pass dtype instance, not dtype class -
Want to keep this PR narrowly scoped.
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen. |
pandas.core.dtypes.common#45417doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.This enables behavior that would previously raise (and often confuse), e.g :