-
Notifications
You must be signed in to change notification settings - Fork 84
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
Do not auto-set timezone, allow date #1886
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1886 +/- ##
==========================================
+ Coverage 91.81% 92.01% +0.20%
==========================================
Files 27 27
Lines 2627 2619 -8
Branches 688 685 -3
==========================================
- Hits 2412 2410 -2
+ Misses 142 139 -3
+ Partials 73 70 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@bendichter What do you think about this approach? |
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.
Looks great!!! Do we still have tests that include timezones? (I'm on my phone)
Great, thanks. I removed the affected tests and added some tests to ensure that the following cases work:
I want to add a few more tests and remove timezone from a few tests, so this is not ready to merge yet. |
ok. Looks great so far! |
I think given a dataset spec or attribute spec with dtype isodatetime, the mapping of a string value to a datetime or date object should occur in HDMF on construction of the object, rather than as a custom constructor argument override on every field that we know has an isodatetime spec. Requiring the override to get the string as a datetime or date object creates a burden on any extension that wants to store datetimes and read datetime data as datetime objects instead of as strings. So, |
Co-authored-by: Steph Prince <[email protected]>
Motivation
Fix #1859, fix #1843
"2024-04-10"
without the "T") can now be roundtripped, so if a user writes adatetime.date
object, it will be written without the "T" and read as adatetime.date
object.Checklist
flake8
from the source directory.