-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Conversation
concat(keys=range)
RangeIndex level in the result
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.
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"]) |
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.
Looks like this evolved from the original test. I don't think we need to be testing both.
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.
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): |
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.
nit: I think you could parametrize with include_none=[True|False]
and then have keys_length = 4 if include_none else 2
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.
Sure thing. Parametrized this test over include_none
instead
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.
lgtm, pending CI 😄
pandas-dev#57755) * PERF/CLN: Preserve RangeIndex level in the result * Whitespace * Whitespace * Fix test * Address review
Enforces #43485
Optimization found in #57441