-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
TST: regression test for GH#56853 (insert/delete with MultiIndex datetime) #62238
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
|
Hi, I've opened a PR which adds a regression test for this issue: #56853. |
|
All checks have passed, I request one of the members to review the changes and kindly merge this PR. Thanks :) |
rhshadrach
left a 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.
Thanks for the PR!
a0fc10b to
fe872e2
Compare
|
Hi @rhshadrach, Thanks for the review! I’ve updated the test as suggested. Please let me know if there’s anything else I should adjust. |
|
@rhshadrach This is ready for a review after the checks are done! |
|
@rhshadrach Its ready for re-review. |
fe872e2 to
7ea8fa6
Compare
|
@rhshadrach Rebased it and all the checks have been passed! |
|
@skalwaghe-56 Thanks for the contribution! Please be patient maintainers will review and provide feedback when they have time. No need to repeatedly request reviews. |
7ea8fa6 to
fb9f350
Compare
|
@Aniketsy Thanks for your comment will wait! |
jorisvandenbossche
left a 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.
Looks good to me, thanks!
6387668 to
9ed8bdc
Compare
Thank you! |
rhshadrach
left a 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.
lgtm
|
Thanks @skalwaghe-56! |
|
Thank you! @rhshadrach |
This PR adds a regression test for a bug where inserting and then deleting
a column in a DataFrame with two-level MultiIndex columns (with a Timestamp
level) used to raise a RecursionError.
The bug is already fixed on main, but this test makes sure it won’t come back.
Test location:
pandas/tests/frame/indexing/test_insert.pyTest method:
TestDataFrameInsert.test_insert_delete_mixed_multiindex_columns