-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Fix DatetimeIndex timezone preservation when joining indexes with same timezone but different units #61234
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
base: main
Are you sure you want to change the base?
Conversation
…h same timezone but different units
Hi @rhshadrach, could you please check this one out? |
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.
@mroeschke - this change will make it so the unit comes out to be the minimum between the two time inputs. Does that look like the correct behavior to you?
# Test union preserves timezone when units differ | ||
result = idx1.union(idx2) | ||
expected = date_range("2000-01-01", periods=3, tz=tz).as_unit("ns") | ||
tm.assert_index_equal(result, expected) | ||
assert result.tz == idx1.tz # Original timezone is preserved |
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.
Can you either break these up into separate tests or parametrize them.
I agree with @rhshadrach's comment on splitting/parametrizing the test, otherwise this LGTM |
Address review comments on PR pandas-dev#60080 by splitting the comprehensive test into separate focused tests for each set operation (union, intersection, symmetric_difference).
result = idx1.union(idx2) | ||
expected = date_range("2000-01-01", periods=3, tz=tz).as_unit("ns") | ||
tm.assert_index_equal(result, expected) | ||
assert result.tz == idx1.tz # Original timezone is preserved |
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.
Can you remove assert result.tz == idx1.tz
throughout; this is already checked by assert_index_equal
.
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.