-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: Arithmetic, timezone and offsets operations affecting to NaT #6873
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
Followings are results after the fix.
|
@@ -611,7 +611,8 @@ def __sub__(self, other): | |||
def _add_delta(self, delta): | |||
if isinstance(delta, (Tick, timedelta)): | |||
inc = offsets._delta_to_nanoseconds(delta) | |||
new_values = (self.asi8 + inc).view(_NS_DTYPE) | |||
new_values = np.array([i + inc if i != tslib.iNaT else i for i in self.asi8]) |
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.
this will be very slow, instead do something like this:
mask = self.asi8 == tslib.iNaT
new_values = (self.asi8 + inc).view(_NS_DTYPE)
new_values[mask] = tslib.iNaT
pls rebase needs a perf check - run a vbench and post/investigate >1.20 ratio |
Followings are vbench results. There seems to be no test constantly get worse, but is there anything should be checked? First time
Second time:
|
if offset is FY5253 or offset is FY5253Quarter: | ||
offset = offset(n=1, startingMonth=1, weekday=1, | ||
qtr_with_extra_week=1, variation='last') | ||
elif offset is WeekOfMonth or offset is LastWeekOfMonth: |
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 yum make a helper function of this offset creation so that u can use it here and in the apply_out_of_range ?
Got it. Modified and rebased. |
BUG: Arithmetic, timezone and offsets operations affecting to NaT
thanks |
NaT affected by some datetime related ops unexpectedly.
Arithmetic
Applying arithmetic ops to
NaT
is not handled properly. Based on numpy results, I understand that results should be allNaT
as long as valid data is passed.Timezone
Closes #5546.
Note I fixed
DatetimeIndex
, and I leaveNatType
still doesn't havetz_localize
andtz_convert
methodsTimestamp
has. Is it should be added?Offsets
These have
apply
method which acceptsTimestamp
, but it cannot handleNat
.