Skip to content
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

Draft: Relax timestamp timezone validation for startDate and endDate fields on NWB file formats #242

Closed
wants to merge 8 commits into from

Conversation

aaronkanzer
Copy link
Member

Relates to dandi/dandi-archive#1944

and changes made for NWB in NeurodataWithoutBorders/pynwb#1886

Observed in failed dandiset validation in staging environment here: https://gui-staging.dandiarchive.org/dandiset/213840

Cc @candleindark @yarikoptic @satra @bendichter

Copy link

codecov bot commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.59%. Comparing base (e135307) to head (40d8fb5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   87.42%   91.59%   +4.17%     
==========================================
  Files          16       16              
  Lines        1726     1726              
==========================================
+ Hits         1509     1581      +72     
+ Misses        217      145      -72     
Flag Coverage Δ
unittests 91.59% <100.00%> (+4.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@satra
Copy link
Member

satra commented May 23, 2024

@aaronkanzer - i'm not sure this is necessary. i believe pydantic default validator for datetime works fine. at least on the example ben sent.

@aaronkanzer
Copy link
Member Author

aaronkanzer commented May 23, 2024

@aaronkanzer - i'm not sure this is necessary. i believe pydantic default validator for datetime works fine. at least on the example ben sent.

Hmmm, agreed -- accordingly to the docs it should work fine (e.g. pydantic handling different formats of datetime), but it is still failing 🤔

Perhaps a different root cause....looking further...

@bendichter
Copy link
Member

Yes, this is what has been confusing us. We don't see any logic in the dandi schema that would seem to require timezones for this field

@aaronkanzer
Copy link
Member Author

Closed for now -- as it doesn't seem that this is the root cause error of validation for certain datetime timestamps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants