-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: pd.DateOffset handle milliseconds #50020
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
Changes from 20 commits
48d9dfa
65cd5da
f989674
b3a5ef1
37427dd
705d2d6
c1be7c6
1788491
ae0b529
64c9ee3
65015c8
ef7f118
b2b7473
c2cf081
3981b04
d80e780
27109a7
76fafe2
2ad8c60
2f913a8
8eb83d0
309e296
0d85a74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -298,43 +298,54 @@ _relativedelta_kwds = {"years", "months", "weeks", "days", "year", "month", | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cdef _determine_offset(kwds): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# timedelta is used for sub-daily plural offsets and all singular | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# offsets, relativedelta is used for plural offsets of daily length or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# more, nanosecond(s) are handled by apply_wraps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kwds_no_nanos = dict( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(k, v) for k, v in kwds.items() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if k not in ("nanosecond", "nanoseconds") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# TODO: Are nanosecond and nanoseconds allowed somewhere? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_kwds_use_relativedelta = ("years", "months", "weeks", "days", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"year", "month", "week", "day", "weekday", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"hour", "minute", "second", "microsecond", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"millisecond") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use_relativedelta = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if len(kwds_no_nanos) > 0: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if any(k in _kwds_use_relativedelta for k in kwds_no_nanos): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if "millisecond" in kwds_no_nanos: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
raise NotImplementedError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Using DateOffset to replace `millisecond` component in " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"datetime object is not supported. Use " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"`microsecond=timestamp.microsecond % 1000 + ms * 1000` " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"instead." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
offset = relativedelta(**kwds_no_nanos) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use_relativedelta = True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# sub-daily offset - use timedelta (tz-aware) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
offset = timedelta(**kwds_no_nanos) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
elif any(nano in kwds for nano in ("nanosecond", "nanoseconds")): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
offset = timedelta(days=0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# GH 45643/45890: (historically) defaults to 1 day for non-nano | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# since datetime.timedelta doesn't handle nanoseconds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
offset = timedelta(days=1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return offset, use_relativedelta | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not kwds: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# GH 45643/45890: (historically) defaults to 1 day | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return timedelta(days=1), False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nanos = {"nanosecond", "nanoseconds"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# nanos are handled by apply_wraps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if all(k in nanos for k in kwds): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return timedelta(days=0), False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kwds_no_nanos = {k: v for k, v in kwds.items() if k not in nanos} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if "millisecond" in kwds_no_nanos: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this could happen earlier, right? and be simplified to |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
raise NotImplementedError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"Using DateOffset to replace `millisecond` component in " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"datetime object is not supported. Use " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"`microsecond=timestamp.microsecond % 1000 + ms * 1000` " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"instead." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. granted, there wasn't a test which hit this to begin with, but there should probably be one? could we add one please (either here or in a separate PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Found these 2, not sure if they qualify as valid test pandas/pandas/tests/tseries/offsets/test_offsets.py Lines 930 to 944 in 8eb83d0
pandas/pandas/tests/tseries/offsets/test_offsets.py Lines 616 to 629 in 8eb83d0
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, thanks! all good then |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kwds_use_relativedelta = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"year", "month", "day", "hour", "minute", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"second", "microsecond", "weekday", "years", "months", "weeks", "days", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"hours", "minutes", "seconds", "microseconds" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# "weeks" and "days" are left out despite being valid args for timedelta, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# because (historically) timedelta is used only for sub-daily. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kwds_use_timedelta = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"seconds", "microseconds", "milliseconds", "minutes", "hours", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if all(k in kwds_use_timedelta for k in kwds_no_nanos): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Sub-daily offset - use timedelta (tz-aware) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# This also handles "milliseconds" (plur): see GH 49897 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return timedelta(**kwds_no_nanos), False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# convert milliseconds to microseconds, so relativedelta can parse it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if "milliseconds" in kwds_no_nanos: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
micro = kwds_no_nanos.pop("milliseconds") * 1000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kwds_no_nanos["microseconds"] = kwds_no_nanos.get("microseconds", 0) + micro | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if all(k in kwds_use_relativedelta for k in kwds_no_nanos): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return relativedelta(**kwds_no_nanos), True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
raise ValueError( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
f"Invalid argument/s or bad combination of arguments: {list(kwds.keys())}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+346
to
349
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a test that hits this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, ever since minutes was added to So it would be something like:
Does it make sense? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah looks good - if we're adding code, let's make sure it's covered |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# --------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Mixins & Singletons | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -1163,7 +1174,6 @@ cdef class RelativeDeltaOffset(BaseOffset): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def __init__(self, n=1, normalize=False, **kwds): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
BaseOffset.__init__(self, n, normalize) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
off, use_rd = _determine_offset(kwds) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
object.__setattr__(self, "_offset", off) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
object.__setattr__(self, "_use_relativedelta", use_rd) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -739,6 +739,28 @@ def test_eq(self): | |
|
||
assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) | ||
|
||
@pytest.mark.parametrize( | ||
"offset_kwargs, expected_arg", | ||
[ | ||
({"microseconds": 1, "milliseconds": 1}, "2022-01-01 00:00:00.001001"), | ||
({"seconds": 1, "milliseconds": 1}, "2022-01-01 00:00:01.001"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should there be a test to check when both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (CI failures are unrelated BTW, if you fetch and merge upstream/main they should be fixed) |
||
({"minutes": 1, "milliseconds": 1}, "2022-01-01 00:01:00.001"), | ||
({"hours": 1, "milliseconds": 1}, "2022-01-01 01:00:00.001"), | ||
({"days": 1, "milliseconds": 1}, "2022-01-02 00:00:00.001"), | ||
({"weeks": 1, "milliseconds": 1}, "2022-01-08 00:00:00.001"), | ||
({"months": 1, "milliseconds": 1}, "2022-02-01 00:00:00.001"), | ||
({"years": 1, "milliseconds": 1}, "2023-01-01 00:00:00.001"), | ||
], | ||
) | ||
def test_milliseconds_combination(self, offset_kwargs, expected_arg): | ||
# GH 49897 | ||
offset = DateOffset(**offset_kwargs) | ||
ts = Timestamp("2022-01-01") | ||
result = ts + offset | ||
expected = Timestamp(expected_arg) | ||
|
||
assert result == expected | ||
|
||
|
||
class TestOffsetNames: | ||
def test_get_offset_name(self): | ||
|
Uh oh!
There was an error while loading. Please reload this page.