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

Issues with recursive data classes #3016

Closed
hgranthorner opened this issue Jun 11, 2021 · 12 comments · Fixed by #3052
Closed

Issues with recursive data classes #3016

hgranthorner opened this issue Jun 11, 2021 · 12 comments · Fixed by #3052
Labels
bug something is clearly wrong here

Comments

@hgranthorner
Copy link

hgranthorner commented Jun 11, 2021

Given the following:

from dataclasses import dataclass
from hypothesis import given, strategies as st

@dataclass
class User():
  id: int
  following: list['User']  # works with typing.List

@given(st.builds(User))
def test_user(u):
    pass

I always receive the error hypothesis.errors.InvalidArgument: thing=User must be a type. I have tried various permutations of using strategies.recursive and strategies.deferred to define custom strategies and pass them to @given, but have been unable to get this to work. I'm not sure if this is a bug or an issue with the documentation (perhaps it requires more recursive examples).

@Zac-HD Zac-HD added the bug something is clearly wrong here label Jun 11, 2021
@Zac-HD
Copy link
Member

Zac-HD commented Jun 11, 2021

Ah, this looks like a bug - typing.List['User'] does work as intended, so we'll just need to track down the difference and ensure that the same logic applies when you use builtins as generics (new in py39).

Thanks for the report - I'm on a camping trip this month, but if nobody else manages to fix it before July I should get to it then 🙂

@hgranthorner
Copy link
Author

Awesome, thanks. I'll try using the old school List until then.

@hgranthorner
Copy link
Author

Just a note: using the old typing.List does work, but is painfully slow. Running the tests above takes about ~7 seconds. I have a hunch recursive is the key to this but the documentation doesn't make it clear how to use it for more complex types.

@sobolevn
Copy link
Member

I will have a look! 👍

@sobolevn
Copy link
Member

I found the reason for this. And as always it is typing API 😒

So, let's see the difference:

from typing import List
from dataclasses import dataclass

@dataclass 
class UserOld:
    following: List['User']

@dataclass 
class UserNew:
    following: list['User']

And now the interesting part:

print(UserOld.__annotations__)
# {'following': typing.List[ForwardRef('User')]}

print(UserNew.__annotations__)
# {'following': list['User']}

Since we treat ForwardRef specially, it is later resolved to a type: typing.List[__main__.User]
But, list['User'] does not get resolved to a similar type, because 'User' is not a forward ref and is just a string.

@Zac-HD what should we do? I know that typing API in python3.9 is different. Should we treat strings in 3.9 the same way as we treat ForwardRef in previous versions?

@Zac-HD
Copy link
Member

Zac-HD commented Jun 27, 2021

Unclear what we could do, since we can't resolve the names in scope if typing failed to do so. Feel free to try, but I think the best we can manage in general might be a more specific error message.

@sobolevn
Copy link
Member

sobolevn commented Jun 27, 2021

It looks like a bug in Python to me, because it does not typecheck type arguments to list:

>>> from typing import List
>>> List[int]  # no error, ok
typing.List[int]
>>> List[1]  # error, ok
Traceback (most recent call last):
  ...
TypeError: Parameters to generic types must be types. Got 1.
>>> list[int]  # no error, ok
list[int]
>>> list[1]   # no error, not ok
list[1]

You can try the same with python@3.9.1

And typing._type_check is where 'User' is converted to ForwardRef('User').
Or probably this is just a new typing API, I am not sure at this point.

PEP does not mention this decision: https://www.python.org/dev/peps/pep-0585/#backwards-compatibility

P.S. Not just list, but other generics as well: set[1], etc

@JelleZijlstra
Copy link

Does this work with from __future__ import annotations?

@sobolevn
Copy link
Member

Does this work with from future import annotations?

No, still fails for the same reason. __annotations__ prop is {'following': "list['User']"}
We correctly handle list as a string, but typing.get_type_hints function fails to resolve 'User' string into User type.

@JelleZijlstra
Copy link

But in the __future__ you can just write list[User], which should hopefully resolve correctly.

@sobolevn
Copy link
Member

Yes, this works:

from __future__ import annotations

from dataclasses import dataclass
from hypothesis import given, strategies as st
from typing import List

@dataclass
class User():
  following: list[User]  # works with typing.List


print(st.from_type(User).example())

@JelleZijlstra thanks a lot for your help!

I guess we need to improve the docs for this error. That's the best we can do.

@Zac-HD
Copy link
Member

Zac-HD commented Aug 7, 2021

I've opened a PR to improve the error message and suggest from __future__ import annotations (instead of using a string) in this case, but that's the most we can do - see https://bugs.python.org/issue41370 for the upstream tracking.

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
4 participants