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

Min/max API consistency #1999

Closed
mhsmith opened this issue Jun 19, 2023 · 2 comments · Fixed by #2005
Closed

Min/max API consistency #1999

mhsmith opened this issue Jun 19, 2023 · 2 comments · Fixed by #2005
Labels
enhancement New features, or improvements to existing features.

Comments

@mhsmith
Copy link
Member

mhsmith commented Jun 19, 2023

The following widgets have a concept of min/max:

  • Slider
  • NumberInput
  • DateInput
  • TimeInput

Affter #1951 is merged, all of them will have min_value and max_value properties, except for Slider, which has a combined range property. For simplicity, I suggest changing all of these to simply min and max, and likewise with the constructor arguments.

This should be done before the next Toga release, to avoid an additional layer of backward compatibility code for DateInput and TimeInput on top of the existing min_date, max_date, etc.

Also, as mentioned at #1951 (comment), NumberInput should be changed so that setting an inconsistent min/max will cause one to clip the other, rather than a ValueError.

@mhsmith mhsmith added the enhancement New features, or improvements to existing features. label Jun 19, 2023
@freakboy3742
Copy link
Member

+1 to the general drive towards consistency, and specifying min/max rather than range.

I can understand the drive behind using the shorter names min and max - my hesitation is the overlap with the builtin methods of the same name. I'm fairly sure we can build implementations that won't be functionally ambiguous, but I'm wondering if we might inadvertently encourage some bad practices in the user base as a result.

@mhsmith
Copy link
Member Author

mhsmith commented Jun 20, 2023

Shadowing the builtin functions wouldn't be an issue for properties, because they're always preceded by a dot. In fact, Slider already has max and min properties, but they're currently read-only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants