Repository: HypothesisWorks/hypothesis
Pull request: Allow subnormal in complex_numbers
Fixes #3390.

Adds the `allow_subnormal` argument to `complex_numbers` by applying it to both the real and imaginary parts separately.

Problem statement:
`allow_subnormal` keyword for `complex_numbers`
I would like to be able to exclude subnormals from the floats drawn (or more specifically the resulting imaginary and real parts of the constructed complex number) in the `complex_numbers` strategy, but that's currently not possible. 
Has this been considered already?

Hints from issue discussion:
Worth noting supporting v2 of the Array API in `hypothesis.extra.array_api`, which includes complex numbers which would presumably make subnormals out-of-scope, would probably want this feature. (I'll have to write a separate issue outlining the different approaches we could take for v2 support generally).
I think we're only missing this because nobody has asked for it before! Very happy to accept a PR from you or @honno; otherwise expect a patch from the scipy sprints.

Just to confirm, I think the sensible way to interpret allow_subnormal=False for complex numbers is that it applies to the real and the imaginary parts, yes? Should make it easy to document and also implement, we just pass it through to the floats strategies internally. 
> Just to confirm, I think the sensible way to interpret allow_subnormal=False for complex numbers is that it applies to the real and the imaginary parts, yes?

I think it's important that the resulting complex number drawn from this strategy has no subnormal real or imaginary part, yes. 

> Should make it easy to document and also implement, we just pass it through to the floats strategies internally.

I think one would need to look at the min/max magnitude constrained code path and check if this is really all that's necessary?
The unconstrained case is definitely trivial.
(self assigning although anyone else feel free to go for it, I'll probably start on this in a few weeks time)
Oh I see, if this is a good sprint task for the US SciPy con coming up then I'm happy to step-back and review! Otherwise yeah will get to it later.
Yeah, if this hasn't been done before the sprints someone will pick it up there. 

> I think one would need to look at the min/max magnitude constrained code path and check if this is really all that's necessary?

We already have [a non-obvious design to make this work for the allow_infinity and allow_nan arguments](https://github.com/HypothesisWorks/hypothesis/blob/master/hypothesis-python/src/hypothesis/strategies/_internal/core.py#L1609) (motivated by nice shrinking), so that really is all it takes!

Commit message: Fix _is_subnormal

Patch to review:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index ae41da8e34..4ea8610e75 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -51,6 +51,7 @@ their individual contributions.
 * `Eduardo Enriquez <https://www.github.com/eduzen>`_ (eduardo.a.enriquez@gmail.com)
 * `El Awbery <https://www.github.com/ElAwbery>`_
 * `Emmanuel Leblond <https://www.github.com/touilleMan>`_
+* `Evan Tey <https://github.com/evantey14>`_
 * `Felix Divo <https://www.github.com/felixdivo>`_
 * `Felix Grünewald <https://www.github.com/fgruen>`_
 * `Felix Sheldon <https://www.github.com/darkpaw>`_
diff --git a/hypothesis-python/RELEASE.rst b/hypothesis-python/RELEASE.rst
new file mode 100644
index 0000000000..b8d77bfc02
--- /dev/null
+++ b/hypothesis-python/RELEASE.rst
@@ -0,0 +1,6 @@
+RELEASE_TYPE: minor
+
+This release adds the ``allow_subnormal`` argument to :func:`~hypothesis.strategies.complex_numbers` by
+applying it to each of the real and imaginary parts separately. Closes :issue:`3390`.
+
+Thanks to Evan Tey for this fix.
diff --git a/hypothesis-python/src/hypothesis/strategies/_internal/core.py b/hypothesis-python/src/hypothesis/strategies/_internal/core.py
index 1f0b5079aa..ca01d1d7cd 100644
--- a/hypothesis-python/src/hypothesis/strategies/_internal/core.py
+++ b/hypothesis-python/src/hypothesis/strategies/_internal/core.py
@@ -1595,6 +1595,7 @@ def complex_numbers(
     max_magnitude: Optional[Real] = None,
     allow_infinity: Optional[bool] = None,
     allow_nan: Optional[bool] = None,
+    allow_subnormal: bool = True,
 ) -> SearchStrategy[complex]:
     """Returns a strategy that generates complex numbers.
 
@@ -1607,6 +1608,9 @@ def complex_numbers(
     is an error to enable ``allow_nan``.  If ``max_magnitude`` is finite,
     it is an error to enable ``allow_infinity``.
 
+    ``allow_subnormal`` is applied to each part of the complex number
+    separately, as for :func:`~hypothesis.strategies.floats`.
+
     The magnitude constraints are respected up to a relative error
     of (around) floating-point epsilon, due to implementation via
     the system ``sqrt`` function.
@@ -1639,7 +1643,14 @@ def complex_numbers(
             f"Cannot have allow_nan={allow_nan!r}, min_magnitude={min_magnitude!r} "
             f"max_magnitude={max_magnitude!r}"
         )
-    allow_kw = {"allow_nan": allow_nan, "allow_infinity": allow_infinity}
+    allow_kw = {
+        "allow_nan": allow_nan,
+        "allow_infinity": allow_infinity,
+        # If we have a nonzero normal min_magnitude and draw a zero imaginary part,
+        # then allow_subnormal=True would be an error with the min_value to the floats()
+        # strategy for the real part.  We therefore replace True with None.
+        "allow_subnormal": None if allow_subnormal else allow_subnormal,
+    }
 
     if min_magnitude == 0 and max_magnitude is None:
         # In this simple but common case, there are no constraints on the
diff --git a/hypothesis-python/tests/cover/test_complex_numbers.py b/hypothesis-python/tests/cover/test_complex_numbers.py
index 107555ffe1..66d19201ab 100644
--- a/hypothesis-python/tests/cover/test_complex_numbers.py
+++ b/hypothesis-python/tests/cover/test_complex_numbers.py
@@ -11,10 +11,12 @@
 import math
 import sys
 
+import pytest
+
 from hypothesis import given, reject, strategies as st
 from hypothesis.strategies import complex_numbers
 
-from tests.common.debug import minimal
+from tests.common.debug import assert_no_examples, find_any, minimal
 
 
 def test_minimal():
@@ -89,3 +91,30 @@ def test_minmax_magnitude_equal(data, mag):
         assert math.isclose(abs(val), mag)
     except OverflowError:
         reject()
+
+
+def _is_subnormal(x):
+    return 0 < abs(x) < sys.float_info.min
+
+
+@pytest.mark.parametrize(
+    "allow_subnormal, min_magnitude, max_magnitude",
+    [
+        (True, 0, None),
+        (True, 1, None),
+        (False, 0, None),
+    ],
+)
+def test_allow_subnormal(allow_subnormal, min_magnitude, max_magnitude):
+    strat = complex_numbers(
+        min_magnitude=min_magnitude,
+        max_magnitude=max_magnitude,
+        allow_subnormal=allow_subnormal,
+    ).filter(lambda x: x.real != 0 and x.imag != 0)
+
+    if allow_subnormal:
+        find_any(strat, lambda x: _is_subnormal(x.real) or _is_subnormal(x.imag))
+    else:
+        assert_no_examples(
+            strat, lambda x: _is_subnormal(x.real) or _is_subnormal(x.imag)
+        )
diff --git a/tooling/ignore-list.txt b/tooling/ignore-list.txt
index ef281d8c9a..ede56a0e41 100644
--- a/tooling/ignore-list.txt
+++ b/tooling/ignore-list.txt
@@ -5,3 +5,4 @@ nin
 strat
 tread
 rouge
+tey


Code review comments:
diff_hunk: @@ -1639,7 +1643,14 @@ def complex_numbers(
             f"Cannot have allow_nan={allow_nan!r}, min_magnitude={min_magnitude!r} "
             f"max_magnitude={max_magnitude!r}"
         )
-    allow_kw = {"allow_nan": allow_nan, "allow_infinity": allow_infinity}
+    allow_kw = {
path: hypothesis-python/src/hypothesis/strategies/_internal/core.py
text: @user1:
OK, looks like Mypy thinks that this includes a non-numeric `min_value` 🙄 

Best solution is probably to just drop a `# type: ignore` on line 1659 below.