Allow for generation of a constrained float with multiple_of argument for hypothesis plugin #2442
Conversation
|
is there any issue for this? |
|
Sorry! I'll make that now |
… float gt/lt with multiple of
tobi-lipede-oodle
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2442 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 5084 5095 +11
Branches 1042 1048 +6
=========================================
+ Hits 5084 5095 +11
Continue to review full report at Codecov.
|
|
@samuelcolvin I didn't add the |
|
may as well include this in v1.8.1 |
|
thanks so much. |
| if exclude_max: | ||
| max_value = max_value - 1 | ||
|
|
||
| return st.integers(min_value, max_value).map(lambda x: x * cls.multiple_of) |
There was a problem hiding this comment.
This strategy may generate integers in the case of multiple_of is an integer. I suggest using st.floats instead.
| if max_value is not None: | ||
| assert max_value >= cls.multiple_of, 'Cannot build model with max value smaller than multiple of' | ||
| max_value = math.floor(max_value / cls.multiple_of) | ||
| if exclude_max: |
There was a problem hiding this comment.
It may be better to use the built-in exclude_max argument to st.floats instead of subtracting, as it reduces the possible range of generated values more than needed.
| if exclude_min: | ||
| min_value = min_value + 1 | ||
| if max_value is not None: | ||
| assert max_value >= cls.multiple_of, 'Cannot build model with max value smaller than multiple of' |
There was a problem hiding this comment.
This assertion restricts the generation more than needed. For example, max_value=-2.0 and multiple_of=-1.0 will fail this assertion, but if the plugin generates -3.0, it satisfies the constraints.
Generally, I suggest taking a look at multipleOf implementation in hypothesis-jsonschema (there are multiple places in the linked file) as it handles all these constraints.
There was a problem hiding this comment.
thanks, will take a look
Change Summary
This enables Hypothesis to generate a
ConstrainedFloatwith themultiple_ofargument enabled.Related issue number
#2443
Checklist
changes/<pull request or issue id>-<github username>.mdfile added describing change(see changes/README.md for details)