-
Notifications
You must be signed in to change notification settings - Fork 148
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
[Feat] Enable temporal selector DatePicker
#309
Conversation
for more information, see https://pre-commit.ci
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.
Hey @nadijagraca - thanks for your work on this! Have a few questions and suggestions :)
vizro-core/src/vizro/models/_components/form/date_range_picker.py
Outdated
Show resolved
Hide resolved
This looks great! Just some answers to your open questions and then I will do a full review next week 🙂
This sounds right to me 👍
Let's go for
Definitely keep it minimal and just expose only the essential arguments like we do for slider/rangeslider. What we should do though is set values for these fields if we think the dmc defaults aren't suitable for us. But those wouldn't be configurable as model fields, just hardcoded into the component returned in the |
Be sure to set the |
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.
Have mostly tidy comments left 🧹 - so ensuring consistency between the other components, docstrings etc. The rest looks good to me 👍
@nadijagraca - could you also update the PR description and title?
vizro-core/changelog.d/20240219_132602_nadija_ratkusic_graca_datepicker.md
Outdated
Show resolved
Hide resolved
vizro-core/changelog.d/20240219_132602_nadija_ratkusic_graca_datepicker.md
Outdated
Show resolved
Hide resolved
vizro-core/src/vizro/models/_components/form/date_range_picker.py
Outdated
Show resolved
Hide resolved
vizro-core/src/vizro/models/_components/form/date_range_picker.py
Outdated
Show resolved
Hide resolved
for more information, see https://pre-commit.ci
…mponents/datepicker
…mponents/datepicker
…mponents/datepicker
Co-authored-by: petar-qb <[email protected]> Co-authored-by: nadijagraca <[email protected]>
…mponents/datepicker
…erage results between different python versions
Description
Added temporal selector
DatePicker
forFilter
andParameter
.DatePicker is a single model, but based on value of argument
range
will either returndmc.DatePicker
ordmc.DateRangePicker
. Default value ofrange
isTrue
and the model will returndmc.DateRangePicker
if range is not provided, or if the selector is not defined.Options where
DateRangePicker
is returned:Filter.selector
but filtercolumn
isdatetime
dtype.vm.DatePicker
asFilter.selector
without specifyingrange
argument or by settingrange=True
Option where
DatePicker
is returned:vm.DatePicker
asFilter.selector
and setsvm.DatePicker
range
argument asFalse
.vm.DatePicker.value
supported format are all inputs that could be translated todatetime.date
. See more hereDone is separate pr:
Screenshot
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":