Skip to content

TST: Testing for mixed int/str Index #61349

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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Conversation

xaris96
Copy link

@xaris96 xaris96 commented Apr 24, 2025

pelagiavlas and others added 26 commits March 26, 2025 22:58
@datapythonista
Copy link
Member

@xaris96 Can you fix the broken tests and remove all your debug files from this PR please?

@datapythonista datapythonista added Testing pandas testing functions or related to the test suite Index Related to the Index class or subclasses labels May 3, 2025
@datapythonista datapythonista changed the title Issue tm TST: Testing for mixed int/str Index May 3, 2025
@datapythonista datapythonista mentioned this pull request May 3, 2025
5 tasks
@xaris96
Copy link
Author

xaris96 commented May 4, 2025

@datapythonista okay. no problem. i would like to ask about the pr. Do we need to fix the failed tests that already existed to pass the pr? we are new to this and need some guidance :)

@datapythonista
Copy link
Member

I think the main idea of the issue is to add the mixed index to the tests, see what it fails, fix the bugs of things that don't work with an index with mix types, and if there are tests that need to be updated, also fix that. Since I think several things will be broken, maybe you can open a first PR with just adding the index with mixed types, then write the failures in a comment to the issue, and then address them in individual PRs. But up to you, whatever you consider it makes things easier. @jbrockmendel anything you'd like to add?

@xaris96
Copy link
Author

xaris96 commented May 5, 2025

@datapythonista thanks for the advise. it is really helpful!

@xaris96
Copy link
Author

xaris96 commented May 5, 2025

@datapythonista also one more question. is there any problem or do we create a mess if we making changes and update this pr? or this is the common tactic?

@datapythonista
Copy link
Member

@datapythonista also one more question. is there any problem or do we create a mess if we making changes and update this pr? or this is the common tactic?

A PR is mainly a UI for your fork's branch. The usual way of working is to just keep updating your fork's branch, until reviewers are happy and it's merged into the project's main branch.

@jbrockmendel
Copy link
Member

anything you'd like to add?

seems like a reasonable approach.

@@ -147,6 +147,11 @@ def test_searchsorted(request, index_or_series_obj):
# See gh-12238
obj = index_or_series_obj

if any(isinstance(x, str) for x in obj) and any(isinstance(x, int) for x in obj):
Copy link
Member

Choose a reason for hiding this comment

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

Can you just do if obj.inferred_type == "mixed"? And everywhere below where you need to xfail this new index

@@ -248,12 +235,21 @@ def test_intersection_base(self, index):

@pytest.mark.filterwarnings(r"ignore:PeriodDtype\[B\] is deprecated:FutureWarning")
def test_union_base(self, index):
if index.inferred_type in ["mixed", "mixed-integer"]:
pytest.skip("Mixed-type Index not orderable; union fails")
Copy link
Member

Choose a reason for hiding this comment

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

Can you use the xfail pattern you used above?

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

Generally could you:

  • use index.inferred_type == "mixed if you need special logic to handle it
  • use xfail over skip if you're expecting the test to fail

@xaris96
Copy link
Author

xaris96 commented May 8, 2025

@mroeschke okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Index Related to the Index class or subclasses Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TST: mixed_int_string Index
5 participants