-
Notifications
You must be signed in to change notification settings - Fork 7
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
Improving the [Date|Time|DateTime]Delta
API
#110
Comments
DateDelta
be normalized, and have one sign?DateDelta
be normalized, and disallow mixed signs?
While I have not encountered the usecases where a delta needs to be the component distinguishing whether it's months or years (this seems to me like something that is defined by the context, not the data object), I am not sure that optimizing for memory at this level and this point is really necessary. That said (devil's advocating here a little bit), it looks like Python's
|
DateDelta
be normalized, and disallow mixed signs?[Date/Time]Delta
API
[Date/Time]Delta
API[Date|Time|DateTime]Delta
API
I would like to chime in: I also think that normalised deltas are expected/assumed coming from the stdlib. Having different behaviours for e.g. Maybe maintaining the denormalised form is out-of-scope for this component? (And could be part of a different module, if necessary?) Rough suggestions to move forward:
|
(If that helps, I'd be happy to try my hand with a few test cases to cover an option I suggested.) |
@bibz thanks for taking the time to chime in. Agree with many of your points. I'm AFK at the moment so will go more in depth in a week or so |
@bibz Agree that it's good to default to 'expected behavior' (i.e. normalization) for users coming from the standard library. However: this only makes sense so far it's possible. The issue here is that while hours/minutes/seconds/milliseconds/etc can be normalized, calendar days and months cannot. To summarize: you always end up with at least 3 "buckets" that can be normalized internally, but not between them:
I'm leaning towards minimizing the 'number of buckets'. The 0.5 release Note that pendulum attempts to put everything into one bucket (using general rules like month=30 days), but that results in strange inconsistencies (see their issue tracker for numerous examples) |
Yes, this makes complete sense. Thank you for the detailed explanation.
That sounds like a very bad decision for anything in production 😱 |
There are two design questions for
DateDelta
:DateDelta
stores years, months, weeks, and days separately. However, in the Gregorian calandar, years are always 12 months, and weeks are always 7 days. It's save on storage to just storemonths
anddays
. The downside is that users may want to deliberately store unnormalized deltas, such that "24 months" isn't the same as "2 years".DateDelta
components may have different signs (P4Y-3M+1W0D
). While this is certainly flexible, it's unclear whether this functionality is actually used. Having one single sign would simplify things—although it may result in exceptions when doing arithmetic (whetheryears(a) + months(b) - years(c)
would raise an exception depending on whether the result would have mixed signs).The text was updated successfully, but these errors were encountered: