-
Notifications
You must be signed in to change notification settings - Fork 629
Fix and deprecate unusual uses of st.recursive()
#4639
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
Conversation
43ab06c to
82b5302
Compare
2e2f484 to
d6911d5
Compare
d6911d5 to
2b6c06f
Compare
|
Noting that there are (very contrived, surely) ways to defeat >>> from hypothesis import strategies as st
>>> st.recursive(st.none(), lambda x: st.lists(locals()['x'])).example()
/Users/tybug/Desktop/Liam/coding/hypothesis/hypothesis-python/src/hypothesis/strategies/_internal/strategies.py:508: HypothesisDeprecationWarning: extend=lambda x: <unknown> doesn't use it's argument, and thus can't actually recurse!
self.do_validate()
[[None], [None, None, None, None, None, None, None, None, None], [], None]
>>> which means I'd lean towards making this a warning, which can be suppressed if we get it wrong, rather than an error after the deprecation period. (Same applies for the |
7658f1b to
a0082e8
Compare
|
"don't do that, then" |
Liam-DeVoe
left a comment
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.
sure, but users will be rightfully annoyed that we presume to know better than them and they can't silence our opinions :)
To be clear I don't view this as blocking. I figured we'd disagree on this; and I still wanted to put my opinion on record here
| status = f"{status} ({data.interesting_origin!r})" | ||
| elif data.status == Status.INVALID and isinstance(data, ConjectureData): | ||
| assert isinstance(data, ConjectureData) # mypy is silly | ||
| status = f"{status} ({data.events.get('invalid because', '?')})" |
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.
unknown might be more clear than ?, but I'll wait to see this in action to form a solid opinion
Closes #4638