Skip to content

CLN: avoid _internal_get_values in pandas._testing #32570

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

Merged
merged 1 commit into from
Mar 11, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions pandas/_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,8 @@ def assert_extension_array_equal(

if hasattr(left, "asi8") and type(right) == type(left):
# Avoid slow object-dtype comparisons
assert_numpy_array_equal(left.asi8, right.asi8)
# np.asarray for case where we have a np.MaskedArray
assert_numpy_array_equal(np.asarray(left.asi8), np.asarray(right.asi8))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can asi8 ever return a MaskedArray?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have one test where we pass a MaskedArray to the constructor (I think DTI)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have one test where we pass a MaskedArray to the constructor (I think DTI)

But shouldn't that be converted to a normal array upon construction?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id have no problem with enforcing this, but AFAIK we dont at the moment

return

left_na = np.asarray(left.isna())
Expand Down Expand Up @@ -1176,20 +1177,23 @@ def assert_series_equal(
raise AssertionError(msg)
elif is_interval_dtype(left.dtype) or is_interval_dtype(right.dtype):
assert_interval_array_equal(left.array, right.array)
elif is_datetime64tz_dtype(left.dtype):
# .values is an ndarray, but ._values is the ExtensionArray.
elif is_categorical_dtype(left.dtype) or is_categorical_dtype(right.dtype):
_testing.assert_almost_equal(
left._values,
right._values,
check_less_precise=check_less_precise,
check_dtype=check_dtype,
obj=str(obj),
)
elif is_extension_array_dtype(left.dtype) or is_extension_array_dtype(right.dtype):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this was "and", and now it is "or". That seems to cause issues in geopandas in case you specify check_dtype=False (in which case you can eg have an EA dtype and object dtype with equal values)

assert_extension_array_equal(left._values, right._values)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert_extension_array_equal doesn't work for categorical?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have kwargs check_categorical and check_category_ordered and a categorical-specific check at the end of assert_series_equal. xref #32571 trying to cut down on these

elif needs_i8_conversion(left.dtype) or needs_i8_conversion(right.dtype):
# DatetimeArray or TimedeltaArray
assert_extension_array_equal(left._values, right._values)
elif (
is_extension_array_dtype(left)
and not is_categorical_dtype(left)
and is_extension_array_dtype(right)
and not is_categorical_dtype(right)
):
assert_extension_array_equal(left.array, right.array)
else:
_testing.assert_almost_equal(
left._internal_get_values(),
right._internal_get_values(),
left._values,
right._values,
check_less_precise=check_less_precise,
check_dtype=check_dtype,
obj=str(obj),
Expand Down