speedate
speedate copied to clipboard
Support explicitly parsing timestamps as seconds/milliseconds
As discussed in https://github.com/pydantic/pydantic/issues/7940, we wish to be able to explicitly specify the unit in which unix timestamsp are to be interpreted.
See comments below for additional context
Codecov Report
Merging #54 (499c5a7) into main (ce08d32) will increase coverage by
0.12%. The diff coverage is100.00%.
Additional details and impacted files
@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 99.09% 99.22% +0.12%
==========================================
Files 6 6
Lines 885 899 +14
==========================================
+ Hits 877 892 +15
+ Misses 8 7 -1
| Files | Coverage Δ | |
|---|---|---|
| src/date.rs | 100.00% <100.00%> (ø) |
|
| src/datetime.rs | 100.00% <100.00%> (ø) |
|
| src/lib.rs | 100.00% <ø> (+8.33%) |
:arrow_up: |
| src/time.rs | 97.82% <100.00%> (+0.04%) |
:arrow_up: |
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update ce08d32...499c5a7. Read the comment docs.
The changes so far are a 'naive' implementation of this idea — simply adding the TimestampUnit enum, and adding it as a parameter wherever it is needed. It turns out it would need to be added in quite some places!
Open questions
- Adding a
TimestampUnitargument to so many methods is obviously a big breaking change. One alternative would be a new 'family' of methods calledparse_str_with_configor similar — assuming other aspects of parsing could become configurable in future. What are your thoughts on this? - On a related note: there was a wrinkle in refactoring the tests, as
DurationandTimedon't require theTimestampUnitparameter. I solved this using a trait, but I wouldn't be surprised it there is a more elegant way that is obvious to more experienced rustacians. - I put the
TimestampUnitenum indate.rssince that's where the timestamp watershed constants were already defined. Does it make sense there?
Also TODO still: updating the docs, once API is approved.
@ariebovenberg maybe I can offer some ideas here (even if @samuelcolvin gets the ultimate decision):
-
regarding
TimestampUnit, I think maybe we should just bite the bullet and haveDateConfigandDateTimeConfigstructures, rather than treat this as an isolated parameter? That'll make it easier to not have breaking changes again in future. -
potentially the tests also get solved by having
parse_strandparse_str_with_configseparation, like we have for time? -
it's fine to have
TimestampUnitindate.rs; the public export isspeedate::TimestampUnitso we can always move this later without users ever knowing.