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
Add handling for from_regex and test for change #3550
Add handling for from_regex and test for change #3550
Conversation
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.
Thanks Jens - for the bug report and also the patch! I think it's pretty close; just want to make sure that we've covered off any 'nearby' issues before merging 🙂
strategy.function is st.from_type | ||
or strategy.function.__name__ is st.from_regex.__name__ |
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.
Maybe strategy.function in (st.from_type, st.from_regex)
?
We'll also want to handle more than just the first argument, in case of e.g. st.from_regex("pattern", flags=re.ASCII)
. If handling flags turns out to be a huge pain I'm happy to just open an issue about that for later, but it'd be good to investigate now in case we can support that with only minor changes.
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.
This sadly does not work, they get different memory addresses somehow, so it needs to be done via .__name__
.
Probably iterating on ._LazyStrategy__kwargs
and _LazyStrategy__args
is the best bet.
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.
Also I just realized the from_regex
does not support flags itself, they need to be included in the pattern after the last /
or in the re.compile
call
(Did a rebase) |
Use function call instead of function Switch to using sets
4c774cb
to
38ee9d3
Compare
New branch and cherry picked |
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.
Lovely, thanks again for finding and fixing this!
Well, if something annoys me and I can change it, I try to change it^^ |
❤️ open source! |
Fixes #3549