Skip to content
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

register_type_strategy doesn't work with lowercase builtin list type #3635

Closed
kasbah opened this issue May 4, 2023 · 1 comment · Fixed by #3675
Closed

register_type_strategy doesn't work with lowercase builtin list type #3635

kasbah opened this issue May 4, 2023 · 1 comment · Fixed by #3675
Labels
bug something is clearly wrong here

Comments

@kasbah
Copy link

kasbah commented May 4, 2023

Using register_type_strategy doesn't seem to work with the built in list type available since python 3.9 (in fact typing.List is deprecated).

hypothesis==6.75.1
pytest==7.3.1
Python 3.10.10

from typing import get_args, Type, List
from hypothesis import strategies as st, given, infer


def _shorter_lists(list_type: Type) -> st.SearchStrategy:
    # doesn't get called unless using capital L List
    print("_shorter_lists called")
    sub_type = get_args(list_type)[0]
    return st.lists(st.from_type(sub_type), max_size=2)

# will make our test fail
st.register_type_strategy(list, _shorter_lists)

# will pass if uncommented, notice capital L
# st.register_type_strategy(List, _shorter_lists)


@given(infer)
def test_example(example: list[int]):
    assert len(example) <= 2
@Zac-HD Zac-HD added the bug something is clearly wrong here label May 4, 2023
@Zac-HD
Copy link
Member

Zac-HD commented Jun 12, 2023

--- a/hypothesis-python/src/hypothesis/strategies/_internal/types.py
+++ b/hypothesis-python/src/hypothesis/strategies/_internal/types.py
@@ -315,7 +315,8 @@ def is_generic_type(type_):
     # We check for `MyClass[T]` and `MyClass[int]` with the first condition,
     # while the second condition is for `MyClass`.
     return isinstance(type_, typing_root_type + (GenericAlias,)) or (
-        isinstance(type_, type) and typing.Generic in type_.__mro__
+        isinstance(type_, type)
+        and (typing.Generic in type_.__mro__ or hasattr(type_, "__class_getitem__"))
     )

This is enough to find the registered strategy for list... as well as for typing.List.

We'll need to do something to ensure that these are treated as a single lookup key, since resolving to different strategies for each will raise ResolutionFailed, but then I think it'll probably work? My main concern is that we'll be left with a more complicated user experience on Python < 3.9, when builtins aren't yet parametrizable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is clearly wrong here
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants