TST: fix tests for mixed int string index #55458
Conversation
natmokval
changed the title
mroeschke
added
the
Testing
pandas/core/sorting.py
Outdated
| try: | ||
| indexer = non_nan_idx[non_nans.argsort(kind=kind)] | ||
| except TypeError as err: | ||
| msg = "'<' not supported between " |
There was a problem hiding this comment.
this looks like the message just cuts off? what is the message in err?
There was a problem hiding this comment.
Thank you for the comments.
Yes, I cut the message off because in tests I had both: "'<' not supported between instances of 'int' and 'str'" and "'<' not supported between instances of 'str' and 'int'" and sometimes messages did not match.
I corrected error message in the function nargsort and tests, now all error messages match.
pandas/tests/base/test_misc.py
Outdated
| @@ -151,6 +151,15 @@ def test_searchsorted(request, index_or_series_obj): | |||
| # comparison semantics https://github.com/numpy/numpy/issues/15981 | |||
| mark = pytest.mark.xfail(reason="complex objects are not comparable") | |||
| request.node.add_marker(mark) | |||
| elif any(isinstance(elem, int) for elem in obj.values[:]) and any( | |||
| isinstance(elem, str) for elem in obj.values[:] | |||
There was a problem hiding this comment.
this is a pretty awkward check. maybe just check obj[0] and obj[1] with a comment the check is written with the mixed-int-string entry in mind?
There was a problem hiding this comment.
agreed, I replaced the check with tm.assert_mixed_int_string_entry
pandas/tests/indexes/test_setops.py
Outdated
| expected = index[1:].set_names(expected_name).sort_values() | ||
| tm.assert_index_equal(intersect, expected) | ||
| if ( | ||
| len(index.values) > 0 |
There was a problem hiding this comment.
maybe a tm.check_for_this_case to de-duplicate all these? or just a new fixture that specifically excludes the relevant case?
There was a problem hiding this comment.
thank you, I added tm.assert_mixed_int_string_entry and replaced the checks with it.
|
@jbrockmendel, I corrected my PR, could you please take a look at the update? |
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
Yes, this PR is still actual. |
|
@jbrockmendel, I updated my PR, could you please take a look at the update? |
pandas/core/sorting.py
Outdated
| 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.
are there cases other than this particular test case that might hit this?
pandas/_testing/asserters.py
Outdated
| @@ -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: | |||
There was a problem hiding this comment.
maybe "is_mixed_..." instead of "assert_mixed_..."?
There was a problem hiding this comment.
Thanks, I replaced assert_mixed_int_string_entry with is_mixed_int_string_entry.
pandas/_testing/asserters.py
Outdated
|
|
||
| def assert_mixed_int_string_entry(arr) -> bool: | ||
| """ | ||
| Check that arr is mixed-int-string entry. |
There was a problem hiding this comment.
I think this will be clearer if it refers directly to the relevant fixture.
There was a problem hiding this comment.
i think this is an instance of a more general pain point in test writing:
def test_something(fixture):
if the_fixture_value_is_this_particular_entry(fixture):
...
usually we either inspect the object like this assert_mixed_int_string_entry is doing or in some cases we pass request and do something hacky with request.fixturenames or something. I wonder if there is a recommended pattern for like pytest.get_active_fixture_id("index") that would return "mixed-int-string" here
There was a problem hiding this comment.
I coud't find a recommended pattern that would return "mixed-int-string" in this case.
Instead I passed request and used request.node.callspec.id to return "mixed-int-string".
Now I inspect the object by using request.node.callspec.id instead of is_mixed_int_string_entry. The function is_mixed_int_string_entry is removed.
Sorry it took so long. Could you please take a look at this PR?
|
I added an entry: |
|
gentle ping @jbrockmendel |
|
Looks like this PR has gone stale and has conflicts now. Closing to clear the queue but if interested in contributing please resolve the conflicts and we can reopen |
Added in conftest's
indices_dictan entry:Adding "mixed-int-string" causes some tests to fail, fixed these tests.