Skip to content

allow open intervals on float range#1303

Merged
davidism merged 2 commits intopallets:masterfrom
robintully:issue_1110_open_interval
Aug 12, 2020
Merged

allow open intervals on float range#1303
davidism merged 2 commits intopallets:masterfrom
robintully:issue_1110_open_interval

Conversation

@robintully
Copy link
Copy Markdown
Contributor

@robintully robintully commented May 6, 2019

fixes #1110

This allows for open intervals (exclusive ranges) for the FloatRange. I believe that without this you would have to specify arbitrary precision of the intervals to emulate open intervals (for example you would have to do 4.999999 in the initialization of FloatRange).

@jcrotts
Copy link
Copy Markdown
Contributor

jcrotts commented May 6, 2019

Looks okay to me. Could you add a changelog entry.

@robintully robintully force-pushed the issue_1110_open_interval branch from edd89cc to b30b909 Compare May 6, 2019 19:34
@ThiefMaster ThiefMaster changed the title [issue 1110] allow open intervals on float range allow open intervals on float range May 6, 2019
@robintully
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, added the changelog

@jcrotts jcrotts added this to the 8.0 milestone May 6, 2019
@davidism
Copy link
Copy Markdown
Member

I think it's clearer as two boolean options, min_inclusive=True, max_inclusive=True. That way it can also be configured to match range, where the start is inclusive but the end is exclusive.

@davidism davidism force-pushed the issue_1110_open_interval branch 2 times, most recently from 50b011a to defc5ec Compare August 11, 2020 03:36
@davidism davidism self-assigned this Aug 11, 2020
@davidism
Copy link
Copy Markdown
Member

davidism commented Aug 11, 2020

I decided the entire implementation of number types needed to be refactored. I've left your initial commit, then added my refactor. min and max can be set to open separately. While open bounds don't mean as much for IntRange, it is implemented for consistency. The clamp behavior was not accounting for open bounds. #1586 by @IamCathal added the range to the help text, but didn't add it for FloatRange, and it needed to be updated for open ranges and unbounded ranges.

I created a _NumberParamTypeBase as the base for IntParamType and FloatParamType, and _NumberRangeBase(_NumberParamTypeBase) as the base for IntRange and FloatRange. All the range behavior except for clamping is implemented by the base class. Help text shows the range like 1<=x<=32, x>=1, x<=32, x<32, etc. For IntRange, clamping is easy for open and closed bounds. For FloatRange, clamping open bounds doesn't make sense, so I'm disallowing that.

Updated the number type tests to test the type directly and use parametrize. Moved the tests to a new test_types.py file. Updated the error messages and docs.

@davidism davidism force-pushed the issue_1110_open_interval branch from defc5ec to c8f92b7 Compare August 11, 2020 14:26
allow specifying min and max as open separately
clamp works for open int bounds, fails for open float bounds
help output shows range information
@davidism davidism force-pushed the issue_1110_open_interval branch from c8f92b7 to d77a0ae Compare August 12, 2020 23:10
@davidism davidism merged commit ca75bab into pallets:master Aug 12, 2020
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FloatRange: add options that exclude each endpoint

3 participants