Skip to content

PERF/CLN: Preserve concat(keys=range) RangeIndex level in the result #57755

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 7 commits into from
Mar 8, 2024

Conversation

mroeschke
Copy link
Member

Enforces #43485

Optimization found in #57441

@mroeschke mroeschke added Performance Memory or execution speed performance Clean Index Related to the Index class or subclasses labels Mar 7, 2024
@mroeschke mroeschke requested a review from rhshadrach as a code owner March 7, 2024 02:00
@mroeschke mroeschke changed the title PERF/CLN: Preserve RangeIndex level in the result PERF/CLN: Preserve concat(keys=range) RangeIndex level in the result Mar 7, 2024
@mroeschke mroeschke added this to the 3.0 milestone Mar 7, 2024
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm, just some nit picks.

@@ -88,22 +88,24 @@ def test_frame_describe_multikey(tsframe):
tm.assert_frame_equal(result, expected)


def test_frame_describe_tupleindex():
@pytest.mark.parametrize("name", ["k", "key"])
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this evolved from the original test. I don't think we need to be testing both.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah cool. Removed

@@ -395,6 +396,44 @@ def test_concat_keys_with_none(self):
expected = concat([df0, df0[:2], df0[:1], df0], keys=["b", "c", "d", "e"])
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize("klass", [range, RangeIndex])
def test_concat_preserves_rangeindex(self, klass):
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think you could parametrize with include_none=[True|False] and then have keys_length = 4 if include_none else 2

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing. Parametrized this test over include_none instead

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm, pending CI 😄

@mroeschke mroeschke merged commit 6a73045 into pandas-dev:main Mar 8, 2024
@mroeschke mroeschke deleted the cln/concat/keys branch March 8, 2024 22:46
pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
pandas-dev#57755)

* PERF/CLN: Preserve  RangeIndex level in the result

* Whitespace

* Whitespace

* Fix test

* Address review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Index Related to the Index class or subclasses Performance Memory or execution speed performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants