address review comments

Author: tybug

hypothesis-python/RELEASE.rst

@@ -1,7 +1,4 @@
 RELEASE_TYPE: patch
 
-Improves several issues surrounding the code introduced in :ref:`version 6.131.1 <v6.131.1>`:
-
-* Improve speed of constants collection.
-* Cache constants to the ``.hypothesis`` directory for future runs.
-* Fix a ``RecursionError`` on parsing deeply nested code.
+This patch makes the new features introduced in :ref:`version 6.131.1 <v6.131.1>` much
+faster, and fixes an internal ``RecursionError`` when working with deeply-nested code.

hypothesis-python/src/hypothesis/internal/conjecture/providers.py

@@ -209,7 +209,7 @@
 _local_modules: set[ModuleType] = set()
 
 
-def _get_local_constants(random: Random) -> "ConstantsT":
+def _get_local_constants() -> "ConstantsT":
     new_constants: set[ConstantT] = set()
     new_modules = list(local_modules() - _local_modules)
     for new_module in new_modules:
@@ -396,8 +396,8 @@ def __init__(self, conjecturedata: Optional["ConjectureData"], /):
 
     @cached_property
     def _local_constants(self):
-        assert self._random is not None
-        return _get_local_constants(self._random)
+        # defer computation of local constants until/if we need it
+        return _get_local_constants()
 
     def _maybe_draw_constant(
         self,

hypothesis-python/src/hypothesis/internal/constants_ast.py

@@ -96,65 +96,52 @@ def visit_Constant(self, node):
         self.generic_visit(node)
 
 
-def _constants_from_source(source: str) -> AbstractSet[ConstantT]:
-    try:
-        tree = ast.parse(source)
-        visitor = ConstantVisitor()
-        visitor.visit(tree)
-    except Exception:
-        # A bunch of things can go wrong here.
-        # * ast.parse may fail on the source code
-        # * NodeVisitor may hit a RecursionError (see many related issues on
-        #   e.g. libcst https://github.com/Instagram/LibCST/issues?q=recursion),
-        #   or a MemoryError (`"[1, " * 200 + "]" * 200`)
-        return set()
-
+def _constants_from_source(source: Union[str, bytes]) -> AbstractSet[ConstantT]:
+    tree = ast.parse(source)
+    visitor = ConstantVisitor()
+    visitor.visit(tree)
     return visitor.constants
 
 
 @lru_cache(4096)
 def constants_from_module(module: ModuleType) -> AbstractSet[ConstantT]:
     try:
         module_file = inspect.getsourcefile(module)
+        # use type: ignore because we know this might error
+        source_bytes = Path(module_file).read_bytes()  # type: ignore
     except Exception:
         return set()
 
-    if module_file is None:
-        return set()
-
+    source_hash = hashlib.sha1(source_bytes).hexdigest()[:16]
+    cache_p = storage_directory("constants") / source_hash
     try:
-        source_bytes = Path(module_file).read_bytes()
+        return _constants_from_source(cache_p.read_bytes())
     except Exception:
-        return set()
-
-    constants_p = (
-        storage_directory("constants") / hashlib.sha1(source_bytes).hexdigest()[:16]
-    )
-    if constants_p.exists():
-        try:
-            source = constants_p.read_text()
-        except Exception:
-            # if there's a problem reading the cached constants, fall back to
-            # standard computation
-            pass
-        else:
-            return _constants_from_source(source)
+        # if the cached location doesn't exist, or it does exist but there was
+        # a problem reading it, fall back to standard computation of the constants
+        pass
 
     try:
-        source = inspect.getsource(module)
+        constants = _constants_from_source(source_bytes)
     except Exception:
+        # A bunch of things can go wrong here.
+        # * ast.parse may fail on the source code
+        # * NodeVisitor may hit a RecursionError (see many related issues on
+        #   e.g. libcst https://github.com/Instagram/LibCST/issues?q=recursion),
+        #   or a MemoryError (`"[1, " * 200 + "]" * 200`)
         return set()
 
-    constants = _constants_from_source(source)
     try:
-        constants_p.parent.mkdir(parents=True, exist_ok=True)
-        constants_p.write_text(
+        cache_p.parent.mkdir(parents=True, exist_ok=True)
+        cache_p.write_text(
             f"# file: {module_file}\n# hypothesis_version: {hypothesis.__version__}\n\n"
-            + str(constants),
+            # somewhat arbitrary sort order. The cache file doesn't *have* to be
+            # stable... but it is aesthetically pleasing, and means we could rely
+            # on it in the future!
+            + str(sorted(constants, key=lambda v: (str(type(v)), v))),
             encoding="utf-8",
         )
-    except Exception:
-        # e.g. read-only filesystem
+    except Exception:  # pragma: no cover
         pass
 
     return constants

hypothesis-python/tests/conjecture/test_local_constants.py

@@ -29,37 +29,29 @@ def test_can_draw_local_constants_integers(monkeypatch, value):
     # _get_local_constants normally invalidates this cache for us, but we're
     # monkeypatching it.
     CONSTANTS_CACHE.cache.clear()
-    monkeypatch.setattr(
-        providers, "_get_local_constants", lambda random: {"integer": {value}}
-    )
+    monkeypatch.setattr(providers, "_get_local_constants", lambda: {"integer": {value}})
     find_any(st.integers(), lambda v: choice_equal(v, value))
 
 
 @xfail_on_crosshair(Why.undiscovered)  # I think float_to_int is difficult for crosshair
 @pytest.mark.parametrize("value", [1.2938, -1823.0239, 1e999, math.nan])
 def test_can_draw_local_constants_floats(monkeypatch, value):
     CONSTANTS_CACHE.cache.clear()
-    monkeypatch.setattr(
-        providers, "_get_local_constants", lambda random: {"float": {value}}
-    )
+    monkeypatch.setattr(providers, "_get_local_constants", lambda: {"float": {value}})
     find_any(st.floats(), lambda v: choice_equal(v, value))
 
 
 @pytest.mark.parametrize("value", [b"abdefgh", b"a" * 50])
 def test_can_draw_local_constants_bytes(monkeypatch, value):
     CONSTANTS_CACHE.cache.clear()
-    monkeypatch.setattr(
-        providers, "_get_local_constants", lambda random: {"bytes": {value}}
-    )
+    monkeypatch.setattr(providers, "_get_local_constants", lambda: {"bytes": {value}})
     find_any(st.binary(), lambda v: choice_equal(v, value))
 
 
 @pytest.mark.parametrize("value", ["abdefgh", "a" * 50])
 def test_can_draw_local_constants_string(monkeypatch, value):
     CONSTANTS_CACHE.cache.clear()
-    monkeypatch.setattr(
-        providers, "_get_local_constants", lambda random: {"string": {value}}
-    )
+    monkeypatch.setattr(providers, "_get_local_constants", lambda: {"string": {value}})
     # we have a bunch of strings in GLOBAL_CONSTANTS, so it might take a while
     # to generate our local constant.
     find_any(