Skip to content

Conversation

@kajarenc
Copy link
Collaborator

@kajarenc kajarenc commented May 8, 2021

Issue: #2731

Now we compute min_value and max_value for the DatePicker widget dynamically based on provided value or values range.

relativedelta used instead of increasing year by 10, to avoid crash when current date is Feb 29 of leap year

[
(date(1961, 4, 12), "1951/04/12", "1971/04/12"),
(date(2020, 2, 29), "2010/02/28", "2030/02/28"),
# TODO: Find a way to mock date.today()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been able to find a way to mock the date.today() function
A possible solution is to use the freezgun library.

Copy link
Collaborator

@vdonato vdonato May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there's an example in the unittest.mock documentation that provides a way to mock date.today: https://docs.python.org/3/library/unittest.mock-examples.html#partial-mocking, which will require a separate test be written since I doubt it'll be possible to combine this approach with using parametrize()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vdonato I tried this option and several options with a modified patch path

Unfortunately, everything falls with errors

For example this:

def test_min_max_values_missing_value(self):
    with patch("streamlit.delta_generator.DeltaGenerator.date_input.date") as mock_date:
        mock_date.today.return_value = date(1999, 10, 8)
        mock_date.side_effect = lambda *args, **kw: date(*args, **kw)

        st.date_input("the label", [])
        c = self.get_delta_from_queue().new_element.date_input
        self.assertEqual(c.min, "1989/10/08")
        self.assertEqual(c.max, "2009/10/08")

fails with error
AttributeError: <function TimeWidgetsMixin.date_input at 0x1526e48b0> does not have the attribute 'date'

I think we will find a way to mock dates in tests. Or we will start using a library for this.

For now, I will leave a TODO comment, to come back to this later.

@kajarenc kajarenc changed the title Fix 2731 Fix inactive DatePicker, If the date value is 10 years earlier May 9, 2021
@kajarenc kajarenc changed the title Fix inactive DatePicker, If the date value is 10 years earlier Fix inactive DatePicker if the date value is 10 years earlier May 9, 2021
@kajarenc kajarenc marked this pull request as ready for review May 9, 2021 18:45
@kajarenc kajarenc requested a review from a team May 9, 2021 18:45
min_value : datetime.date or datetime.datetime
The minimum selectable date. Defaults to today-10y.
The minimum selectable date. Defaults to value - 10 years or left
boundary of interval - 10 years, if value is а range.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pay attention to the docstring.

Do we need to update the documentation somewhere else?
Does this docstring line explain the behavior well enough, or is it better to rephrase?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should be updated automatically (as long as this docstring is already in the existing docs, which it is) since docs in our API reference are rendered from the docstrings themselves.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the documentation wording, I think this is fine, but it may be slightly clearer to phrase it as something like

"The minimum selectable date. If value is a date, defaults to value - 10 years. If value is the interval [start, end], defaults to start - 10 years."

[
(date(1961, 4, 12), "1951/04/12", "1971/04/12"),
(date(2020, 2, 29), "2010/02/28", "2030/02/28"),
# TODO: Find a way to mock date.today()
Copy link
Collaborator

@vdonato vdonato May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there's an example in the unittest.mock documentation that provides a way to mock date.today: https://docs.python.org/3/library/unittest.mock-examples.html#partial-mocking, which will require a separate test be written since I doubt it'll be possible to combine this approach with using parametrize()

min_value : datetime.date or datetime.datetime
The minimum selectable date. Defaults to today-10y.
The minimum selectable date. Defaults to value - 10 years or left
boundary of interval - 10 years, if value is а range.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should be updated automatically (as long as this docstring is already in the existing docs, which it is) since docs in our API reference are rendered from the docstrings themselves.

min_value : datetime.date or datetime.datetime
The minimum selectable date. Defaults to today-10y.
The minimum selectable date. Defaults to value - 10 years or left
boundary of interval - 10 years, if value is а range.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the documentation wording, I think this is fine, but it may be slightly clearer to phrase it as something like

"The minimum selectable date. If value is a date, defaults to value - 10 years. If value is the interval [start, end], defaults to start - 10 years."

@kajarenc kajarenc merged commit e52fc33 into streamlit:develop May 11, 2021
tconkling added a commit to tconkling/streamlit that referenced this pull request May 11, 2021
* develop:
  Fix inactive DatePicker if the date value is 10 years earlier (streamlit#3241)
  Upgrade trim to 0.0.3 or later (streamlit#3250)
  Manually name all snapshots and generate missing ones (streamlit#3205)
tconkling added a commit that referenced this pull request May 18, 2021
* develop: (75 commits)
  Remove tag from PR template (#3284)
  Add test req version bound for TF to fix tests (#3266)
  pin click to < 8.0 (#3256)
  Up version to 0.82.0
  Update change log
  Add support for toml syntax highlighting in code blocks (#3140)
  Don't allow config.get_option to be called on file import (#3235)
  pin click to < 8.0 (#3256)
  Random cleanup: docstrings + type-safety (#3252)
  "setWidgetValue" -> "commitWidgetValue" (#3253)
  Fix inactive DatePicker if the date value is 10 years earlier (#3241)
  Upgrade trim to 0.0.3 or later (#3250)
  Manually name all snapshots and generate missing ones (#3205)
  Fix Video Recorded modal dialog width (#3226)
  Bump handlebars from 4.7.6 to 4.7.7 in /frontend (#3238)
  Bump hosted-git-info from 2.8.8 to 2.8.9 in /frontend (#3234)
  Bump lodash from 4.17.20 to 4.17.21 in /frontend (#3233)
  Bump ua-parser-js from 0.7.23 to 0.7.28 in /frontend (#3231)
  Update FAQs with supported browsers (#3225)
  Update copyright date (#3228)
  ...
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