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

Library is not compatible with "from __future__ import annotations" (PEP 563) #62

Closed
chrahunt opened this issue Feb 10, 2019 · 2 comments

Comments

@chrahunt
Copy link

chrahunt commented Feb 10, 2019

PEP 563 means that we cannot expect type annotations to actually be class objects. Example code that fails (Python 3.7):

from __future__ import annotations

from dataclasses import dataclass
from dataclasses_json import dataclass_json


@dataclass
class T:
    f: str

@dataclass_json
@dataclass
class U:
    t: T

obj = U(T('foo'))
print(obj)
s = obj.to_json()
print(s)
obj2 = U.from_json(s)
print(obj2)

which outputs

U(t=T(f='foo'))
{"t": {"f": "foo"}}
U(t={'f': 'foo'})

The expected behavior is that t is of type T and not a dict. This works as expected when from future import __annotations__ is commented out.

@lidatong
Copy link
Owner

Hi, yes, I am aware of this issue and it is being tracked in #31, specifically #5. Thanks for reporting.

My instinct is there isn't a good way to address this (as described in #5) without improvements to the typing API, but let me know if you have suggestions.

@chrahunt
Copy link
Author

To get the types we must call typing.get_type_hints and passing the class. This returns a mapping that can be used to retrieve the type by passing field.name. For example:

$ python
Python 3.7.1 (default, Jan  1 2019, 15:19:53)
[GCC 5.5.0 20171010] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from __future__ import annotations
>>> from dataclasses import dataclass, fields
>>> import typing
>>> @dataclass
... class C:
...  i: int
...
>>> fields(C)
(Field(name='i',type='int',default=<dataclasses._MISSING_TYPE object at 0x7f62e8594780>,default_factory=<dataclasses._MISSING_TYPE object at 0x7f62e8594780>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=_FIELD),)
>>> fields(C)[0].type
'int'
>>> fields(C)[0].name
'i'
>>> typing.get_type_hints(C)
{'i': <class 'int'>}
>>> typing.get_type_hints(C)[fields(C)[0].name]
<class 'int'>

The only caveat with this approach is that for classes that use forward references (with the class name itself) the typing.get_type_hints will raise a NameError if called before the class has been actually initialized in its module (such as directly in a decorator).

facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Feb 14, 2023
Summary:
There's an [issue](lidatong/dataclasses-json#62) with dataclasses_json supporting `from __future__ import annotations`. This diff:
- Removes `from __future__ import annotations`
- adds `mm_field` argument to `dataclasses.field`

The second bullet point seems like it wouldn't be necessary but it was the only thing that got the error message to go away. It seems like it fixed the false negative on `ValidationError`s as well.

Alternative options to this would be:
- remove reliance on dataclasses_json
This is a long term solution that we might want especially when newer python versions implicitly include `from __future__ import annotations`. It requires reworking the entirety of `code_navigation_request` so I am leaving it for the future.
- Fully rely on `dataclasses_json`
We can't do this that well because we need to override certain json encodings because of the protocol.

Reviewed By: stroxler, grievejia

Differential Revision: D43201933

fbshipit-source-id: 29e769a4a3c2c50c031a6602c7ff786593978b1b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants