API: register_extension_dtype class decorator #22666
Conversation
TomAugspurger
added
the
ExtensionArray
|
Hello @TomAugspurger! Thanks for submitting the PR.
|
TomAugspurger
force-pushed
the
ea-register
branch
from
98ac047 to
b1aac08
Compare
| @@ -51,10 +50,6 @@ def test_eq_with_numpy_object(self, dtype): | |||
| def test_array_type(self, data, dtype): | |||
| assert dtype.construct_array_type() is type(data) | |||
|
|
|||
| def test_array_type_with_arg(self, data, dtype): | |||
There was a problem hiding this comment.
This test could never pass or fail correctly. It would always raise a TypeError since the method doesn't take an argument.
| @@ -77,9 +77,7 @@ def data_for_grouping(): | |||
|
|
|||
|
|
|||
| class TestDtype(base.BaseDtypeTests): | |||
|
|
|||
| def test_array_type_with_arg(self, data, dtype): | |||
There was a problem hiding this comment.
What's it testing? That code wasn't ever run on master.
There was a problem hiding this comment.
What do you mean with "That code wasn't ever run on master." ? That the test is never run, or that what it is testing is never used anywhere in the code?
Those tests are certainly running, but it is true that the base class test is not very useful (and also wrong with the string argument).
But I think it is still potentially useful to assert that the dtypes construct_array_type is giving an ExtensionArray class object
There was a problem hiding this comment.
The test was overridden by every subclass. Note that we still have https://github.com/pandas-dev/pandas/pull/22666/files/9b646e28656ca3c61ee8a221e16ad74bba2610c3#diff-32e4b328fc01507825a6249caac0cb21R50, which tests this properly.
There was a problem hiding this comment.
Note that we still have .., which tests this properly.
Ah, yes, that is indeed what the other overridden ones were also doing, but then manually.
|
also can you change the registration of the pandas EA types to use the new register function (e.g. Categorical, Interval etc) |
Codecov Report
@@ Coverage Diff @@
## master #22666 +/- ##
==========================================
+ Coverage 92.17% 92.17% +<.01%
==========================================
Files 169 169
Lines 50708 50717 +9
==========================================
+ Hits 46740 46749 +9
Misses 3968 3968
Continue to review full report at Codecov.
|
|
Updated for categorical and interval. Couldn't do the same for the various Integer dtypes, since they're generated dynamically. |
https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/integer.py#L621 just a 1-liner to change |
|
Right... fixed. |
|
All green (this was split from the SparseArray PR fyi. I think I have one or two more things to split before) |
|
lgtm though i think should re-evaluate the test (could be that wasn’t testing anything), but this should have a test |
Closes #22664