-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Provide ExtensionDtype.construct_from_string by default #26562
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
Changes from 7 commits
20ec4e4
abb5eb1
661d522
8ff1965
609910c
a50301a
e04ed79
ef2d005
54cca4f
7fa61d2
2897ad2
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 |
---|---|---|
|
@@ -51,6 +51,36 @@ def test_pickle(self): | |
assert result == self.dtype | ||
|
||
|
||
class TestBaseDtype: | ||
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. Such tests are already in the base extension tests (tests/extension/base/dtype), or can be expanded there 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 had the feeling that those were only base tests for implementing tests for custom extensions, couldn't find a place where I thought it'd make sense to have the abstract class test. Where exactly would you have them? 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 have custom extensions ourselves, so we use those tests ourselves (and in addition we also have a few extra test EAs). For example in
What do you mean with abstract class test? 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 don't think I understand what you want to test. @jreback wanted a test for the existence of the method I saw that in I don't think we want to test that the method is implemented in the
Am I missing something here? Sorry I don't understand exactly what you propose. 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. see @jorisvandenbossche comments on where tests should go, we have a very general framework for these already. |
||
def setup_method(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. we really don't want to use setup_method generally at all. |
||
class DummyDtype(pd.core.dtypes.base.ExtensionDtype): | ||
name = 'dummy' | ||
self.dtype = DummyDtype() | ||
|
||
def test_str(self): | ||
assert str(self.dtype) == self.dtype.name | ||
|
||
def test_eq(self): | ||
assert self.dtype == 'dummy' | ||
assert self.dtype != 'anonther_type' | ||
|
||
def test_default_kind(self): | ||
assert self.dtype.kind == 'O' | ||
|
||
def test_construct_from_string(self): | ||
dtype_instance = self.dtype.__class__.construct_from_string('dummy') | ||
assert isinstance(dtype_instance, self.dtype.__class__) | ||
with pytest.raises(TypeError, match="Cannot construct a 'DummyDtype' " | ||
"from 'another_type'"): | ||
self.dtype.__class__.construct_from_string('another_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. I think this would be a useful test to add to the base extension tests, and the |
||
|
||
def test_default_is_numeric(self): | ||
assert not self.dtype._is_numeric | ||
|
||
def test_default_is_boolean(self): | ||
assert not self.dtype._is_numeric | ||
|
||
|
||
class TestCategoricalDtype(Base): | ||
|
||
def create(self): | ||
|
@@ -82,7 +112,7 @@ def test_equality(self): | |
def test_construction_from_string(self): | ||
result = CategoricalDtype.construct_from_string('category') | ||
assert is_dtype_equal(self.dtype, result) | ||
msg = "cannot construct a CategoricalDtype" | ||
msg = "Cannot construct a 'CategoricalDtype' from 'foo'" | ||
with pytest.raises(TypeError, match=msg): | ||
CategoricalDtype.construct_from_string('foo') | ||
|
||
|
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.
Does anybody know why we had this "except AttributeError: pass" here?