-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
TST: fix tests for mixed int string index #55458
Changes from 10 commits
9b8c639
3645428
6b0e2c2
786c250
533b4a2
4ad1bc4
87160c0
a27bc13
2d67a27
531beda
f09b0ac
adf4929
fac6291
5eef4fc
8a487e3
5fb2f55
3081ad5
e7edbbe
14d6b6f
454f0b6
82ad988
72435c6
7ebd7ef
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 |
---|---|---|
|
@@ -1382,3 +1382,20 @@ def assert_metadata_equivalent( | |
assert val is None | ||
else: | ||
assert val == getattr(right, attr, None) | ||
|
||
|
||
def assert_mixed_int_string_entry(arr) -> bool: | ||
""" | ||
Check that arr is mixed-int-string entry. | ||
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 will be clearer if it refers directly to the relevant fixture. 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 is an instance of a more general pain point in test writing:
usually we either inspect the object like this assert_mixed_int_string_entry is doing or in some cases we pass 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 coud't find a recommended pattern that would return Now I inspect the object by using |
||
""" | ||
# we might have an entry [0, "a", 1, "b", 2, "c"] with duplicates | ||
# or with None or without the first element | ||
if len(arr) < 3: | ||
return False | ||
else: | ||
if isinstance(arr[0], int): | ||
return isinstance(arr[1], str) | ||
elif isinstance(arr[0], str): | ||
return isinstance(arr[1], int) | ||
else: | ||
return isinstance(arr[2], int) and isinstance(arr[3], str) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -436,7 +436,13 @@ def nargsort( | |
if not ascending: | ||
non_nans = non_nans[::-1] | ||
non_nan_idx = non_nan_idx[::-1] | ||
indexer = non_nan_idx[non_nans.argsort(kind=kind)] | ||
# GH#54072 | ||
# argsort does not support mixed int/string Index | ||
try: | ||
indexer = non_nan_idx[non_nans.argsort(kind=kind)] | ||
except TypeError as err: | ||
msg = "'<' not supported between instances of 'int' and 'str'" | ||
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. are there cases other than this particular test case that might hit this? |
||
raise TypeError(msg) from err | ||
if not ascending: | ||
indexer = indexer[::-1] | ||
# Finally, place the NaNs at the end or the beginning according to | ||
|
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.
maybe "is_mixed_..." instead of "assert_mixed_..."?
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.
Thanks, I replaced
assert_mixed_int_string_entry
withis_mixed_int_string_entry
.