-
-
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
ENH/TST: Add BaseMethodsTests tests for ArrowExtensionArray #47552
Conversation
mroeschke
commented
Jun 30, 2022
•
edited
Loading
edited
- Tests added and passed if fixing a bug or adding a new feature
- All code checks passed.
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.
lgtm. cc @jbrockmendel for a quick glance
Will take a look today (and start whittling down the ping backlog) |
@@ -49,8 +50,7 @@ def test_value_counts_with_normalize(self, data): | |||
else: | |||
expected = pd.Series(0.0, index=result.index) | |||
expected[result > 0] = 1 / len(values) | |||
|
|||
if isinstance(data.dtype, pd.core.dtypes.dtypes.BaseMaskedDtype): | |||
if na_value_for_dtype(data.dtype) is pd.NA: |
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.
not for this PR, but we probably need to find a way to let authors define na_value_for_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.
Probably similar to #27825
super().test_container_shift(data, frame, periods, indices) | ||
|
||
@pytest.mark.xfail( | ||
reason="result dtype pyarrow[bool] better than expected dtype object" |
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.
medium-term should just override the test with the correct/better behavior
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.
LGTM
…ev#47552) * ENH/TST: Add BaseMethodsTests tests for ArrowExtensionArray * Passing test now * add xfails for arraymanager * Fix typo * Trigger CI * Add xfails for min version and datamanger * Adjust more tests