-
Notifications
You must be signed in to change notification settings - Fork 915
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
Delay materializing RangeIndex in .reset_index #15588
Delay materializing RangeIndex in .reset_index #15588
Conversation
…ndex/ri/materialize
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.
Nothing jumps out for me while looking at the code but @mroeschke can you look into why there are new failures in the following files:
- tests/generic/test_finalize.py
- tests/frame/methods/test_to_period.py
- tests/frame/methods/test_rank.py
It might be noise but wanted to be sure if these failures are new because of this PR or not.
…ndex/ri/materialize
Did you see these failures locally? Looks like the CI didn't pick up these failures |
@galipremsagar could you comment on where you saw those failures? |
They were here: https://github.com/rapidsai/cudf/actions/runs/8823446932/attempts/1#summary-24226360963 But after looking at my PRs, these failures are probably un-related. So ignore my comment. |
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.
Based on Prem's comment above, approving assuming that my understanding in the one question that I raise is correct.
/merge |
Description
Before, a
RangeIndex
would be materialized to aColumn
even if it wasn't used (drop=True
). Now, it's only materialized if the index will be added as a new column in the DataFrame.Also caught a validation bug where an
invalid
number of levels would not raise an errorChecklist