-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH/BUG: Period/PeriodIndex supports NaT #7485
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
|
||
def __add__(self, other): | ||
return PeriodIndex(ordinal=self.values + other, freq=self.freq) | ||
try: | ||
mask = self.values == tslib.iNaT |
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.
these looking similar, maybe use a helper function?
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.
Yep, fixed.
|
||
result = getattr(self.values, opname)(other.values) | ||
|
||
mask = np.ma.mask_or(self.values == tslib.iNaT, other.values == tslib.iNaT) |
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.
why not use|
here? or com.mask_missing
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.
Because this is taking elemental-wise logical or. This is for the case like
idx1 = pd.PeriodIndex(['2011-01', 'NaT'], freq='M')
idx2 = pd.PeriodIndex(['NaT', '2011-01'], freq='M')
idx1 > idx2
# [False False]
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.
?
In [56]: idx1 = pd.PeriodIndex(['2011-01', '2011-02'], freq='M')
In [59]: idx2 = pd.PeriodIndex(['2011-01', '2011-03'], freq='M')
In [60]: idx1>idx2
Out[60]: array([False, False], dtype=bool)
In [61]: idx2>idx1
Out[61]: array([False, True], dtype=bool)
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 may misunderstand the point, but I intend to the PeriodIndex
comparison to be compat with Timestamp/NaT
behavior.
pd.NaT > pd.Timestamp('2011-01-01')
# False
pd.NaT < pd.Timestamp('2011-01-01')
# False
But I now found DatetimeIndex
behaves oddly. Maybe the correct result is [False, False]
if compared elemental-wise.
idx1 = pd.DatetimeIndex(['2011-01-01', pd.NaT], freq='M')
idx2 = pd.DatetimeIndex([pd.NaT, '2011-01-01'], freq='M')
print(idx1 > idx2)
# [True, False]
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.
yes that last example should be [False False]
, a NaT
comparison should always be False (like a NaN one). I just wasn't sure why you are using a numpy masked array function (I tend to avoid those, they seem a bit opaque), and prob have something in core/common in any event.
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, modified. I'll issue DatetimeIndex
problem separatelly.
looks fine, ping when green |
@jreback Thanks, get green! |
ENH/BUG: Period/PeriodIndex supports NaT
excellent thanks! |
Closes #7228. Closes #4731.
Period
andPeriodIndex
now can containNaT
usingiNaT
as its internal value.