-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
ENH/BUG: DatetimeIndex and PeriodIndex in-place ops behaves incorrectly #7741
Conversation
This can solve #3367. Test cases are added. |
@@ -729,6 +825,86 @@ def test_representation(self): | |||
result = getattr(idx, func)() | |||
self.assertEqual(result, expected) | |||
|
|||
def test_add_iadd(self): | |||
# union |
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 add a couple of tests that have Periods with another freq as well?
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.
OK, added few more test both for addition and subtraction. There seems to be a problem for non-monotonic PeriodIndex.union
and diff
. I'll dig in...
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.
I meant like adding 'M' (higher than day freq). lmk
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.
OK. Added tests for 'M' and 'A'.
look good..thanks |
ENH/BUG: DatetimeIndex and PeriodIndex in-place ops behaves incorrectly
Fixes 2 issues related to
DetetimeIndex
andPeriodIndex
ops.PeriodIndex
raiseTypeError
(Closes BUG: combine_first behaves strangely with period index #3367).PeriodIndex
in-place operation results inInt64Index
(Closes Inplace operation on PeriodIndex cast it to Int64Index #6527)