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

Datetime widget fixes #3581

Merged
merged 8 commits into from
Sep 2, 2022
Merged

Conversation

vidartf
Copy link
Member

@vidartf vidartf commented Sep 1, 2022

This was missed in previous PR.
While hard to reproduce, there has been some reports about this being an issue, so we make an explicit check to cover this case.
@github-actions
Copy link

github-actions bot commented Sep 1, 2022

Binder 👈 Launch a binder notebook on branch vidartf/ipywidgets/datetime-fix

@jasongrout
Copy link
Member

Thanks. Can you add documentation in the Widget List page for NaiveDateTime?

I was looking at the api, and I think arguably the default date/time picker probably should be the naive one, just like the generic python date/time is naive by default. I think the datetime picker is most often used for filtering historical data, and I think generally users want to pick date/times according to the source data's convention. In other words, if I'm in timezone +5, and I select data from midnight of a certain date to midnight of another date, I think it is most likely I mean midnight of the data's date, not midnight in whatever timezone I happen to be in (i.e., I most likely am wanting the naive date/time). However, we've already shipped 8.0, so perhaps that ship has sailed, and we should just really clearly document the differences between the two, and note when a person might want to use one vs another.

Clean up some test and variable names and extract some common value for readability
Also fixes the none check in time validation
@jasongrout-db
Copy link

However, we've already shipped 8.0, so perhaps that ship has sailed

Also, I didn't realize this matches the api in ipydatetime (of course, since you authored both :), so in a sense it is backwards compatible with this api. So yeah, I think just adding more explanation about the NaiveDateTimePicker in the docs, and when you would want to use it vs the DateTimePicker, would be great.

@vidartf
Copy link
Member Author

vidartf commented Sep 2, 2022

I ported the docs over from ipydatetime (simplified the code).

@jasongrout jasongrout merged commit 3a60698 into jupyter-widgets:master Sep 2, 2022
@jasongrout
Copy link
Member

Thanks!

@vidartf vidartf deleted the datetime-fix branch September 2, 2022 17:32
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.

AttributeError raised by DatetimePicker
3 participants