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

Allow validating timezone with Inspector calls #831

Merged
merged 12 commits into from
Jun 5, 2024

Conversation

garrettmflynn
Copy link
Member

Uses NeurodataWithoutBorders/nwbinspector#471 to properly validate timezone with Inspector checks.

@garrettmflynn garrettmflynn self-assigned this Jun 5, 2024
@CodyCBakerPhD
Copy link
Collaborator

Would you recommend this get merged before or after the parent?

@garrettmflynn
Copy link
Member Author

Hmm depends on your answer to this question. With any timezone awareness, we have to decide how we want to limit the user's selection of future dates—if at all.

Right now, the date selector uses the current time in local time—but this isn't timezone aware so, for instance, I would not be able to choose a future time tomorrow for a project run in Europe.

This is a general problem across both PRs. Though the consequences are limited by Ben's earlier note that data from different timezones will rarely be converted the same day.

@CodyCBakerPhD
Copy link
Collaborator

Yeah that's really an edge case so I'll check this out in more detail tomorrow

Base automatically changed from timezone to main June 5, 2024 15:25
@CodyCBakerPhD CodyCBakerPhD enabled auto-merge June 5, 2024 15:51
@CodyCBakerPhD CodyCBakerPhD merged commit ced73f2 into main Jun 5, 2024
21 of 22 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the inspector-timezone-validation branch June 5, 2024 16:02
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.

2 participants