-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
WIP/ENH: negative and large datetimes #15148
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
if not isinstance(vmax, _datetimey): | ||
vmax = _datetimey._datetime_to_datetimey(vmax, 0) | ||
vmind, vmaxd = _datetimey._datetimey_to_datetime_samey0(vmin, vmax) | ||
self.rule.set(dtstart=vmind, until=vmaxd) |
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.
Note that in the old code dstart=vmin-(vmax-vmin)
for some reason, and dstop = vmax + (vmax - vmin)
. Expanding the range by a factor of three doesn't make much sense to me. The test that changed (test_DateFormatter
), in my opinion, changed for the better. So this is a mildly breaking change, but it gives far more consistent results.
@@ -206,6 +207,151 @@ def _get_rc_timezone(): | |||
MO, TU, WE, TH, FR, SA, SU) | |||
WEEKDAYS = (MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY, SATURDAY, SUNDAY) | |||
|
|||
class _datetimey(datetime.datetime): |
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.
my guess is that this whole class should go in its own file...
Is this still relevant, now that we can set the epoch? |
Yeah, because it works around datetime's issues with years > 9999 and less than 0. |
PR Summary
This is a WIP, but for discussion as to whether this is worth the extra code.
It is easy to make negative datetime64 objects, or large datetime64 objects that are greater than 10000 years, whereas right now we restrict to between year 1 and 9999 because we use
datetime.datetime
.The obvious solution is to use
datetime64
, but that falls down because a) we want to be able to deal with timezones, anddatetime64
doesn't by default. b) we want to usedateutil.rrule
for things like "put a tick every other Wednesday".So, the solution proposed here is to derive a subclass of
datetime
that keeps track of more years than 1-9999 (I'm not actually clear whydatetime
restricts to these values in the first place). I've attempted (probably poorly) to make the subclass as minimal as possible, and to call the super as often as possible.Before
This PR:
PR Checklist