-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
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
Changes from all commits
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 |
---|---|---|
|
@@ -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)) | ||
return | ||
|
||
left_na = np.asarray(left.isna()) | ||
|
@@ -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): | ||
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. Before this was "and", and now it is "or". That seems to cause issues in geopandas in case you specify |
||
assert_extension_array_equal(left._values, right._values) | ||
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. assert_extension_array_equal doesn't work for categorical? 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 have kwargs |
||
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), | ||
|
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.
How can
asi8
ever return a MaskedArray?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.
we have one test where we pass a MaskedArray to the constructor (I think DTI)
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.
But shouldn't that be converted to a normal array upon construction?
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.
id have no problem with enforcing this, but AFAIK we dont at the moment