Add width to hypothesis.strategies.complex_numbers()
#3559
Conversation
Previously, it was not clear from the documentation how these values are treated. Although the actual implementation is more complicated, I think this description (referring the users to `hypothesis.strategies.floats`) is sufficient.
There was a problem hiding this comment.
Nice work!
I think I'm not sure on width here. Maybe component_width would bring greater clarity to what a width argument would be (in that case, the width of the real/imag components individually)... but I'm not sure on that, so would wait to see what others think.
Valid point, I thought about that too. However, I figuered that sticking to the notation of numpy (and therby probably most other libraries that came after) is a good idea, to not confuse things. |
Co-authored-by: Matthew Barber <quitesimplymatt@gmail.com>
Co-authored-by: Matthew Barber <quitesimplymatt@gmail.com>
Co-authored-by: Matthew Barber <quitesimplymatt@gmail.com>
|
That the correct widths are computed is already checked by the |
There was a problem hiding this comment.
That the correct widths are computed is already checked by the
floats()tests, I'd say the current tests are therefore enough. But if not (there could be a bug in computingcomplex128-> 2xfloat64): Where would I add a test for that/where is that tested forfloats()?
So st.floats() looks to have a ton of tests for things like allow_infinity that we don't also repeat for st.complex_numbers() (i.e. in tests/cover/test_float_nastiness.py). With that in mind, yeah I think the only tests we really need are covering possibilities that can happen in specifically st.complex_numbers(). Feel free to take a stab (good start already), but I'll mull this over myself too.
Co-authored-by: Matthew Barber <quitesimplymatt@gmail.com>
There was a problem hiding this comment.
Looks great - thanks Felix!
I merged in #3557 so we'd get them in the same release, and tweaked the changelog (since we're adding a new argument, it's a minor release). Since I'm about to go to bed, we'll auto-merge and release if CI passes 🚀
|
That's awesome! |
Towards #3468