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

get_type_hints fails if there are un-annotated fields in a dataclass #82129

Open
a-recknagel mannequin opened this issue Aug 26, 2019 · 9 comments
Open

get_type_hints fails if there are un-annotated fields in a dataclass #82129

a-recknagel mannequin opened this issue Aug 26, 2019 · 9 comments
Assignees
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes topic-typing type-bug An unexpected behavior, bug, or error

Comments

@a-recknagel
Copy link
Mannequin

a-recknagel mannequin commented Aug 26, 2019

BPO 37948
Nosy @ericvsmith, @serhiy-storchaka, @ilevkivskyi, @a-recknagel

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/ericvsmith'
closed_at = None
created_at = <Date 2019-08-26.06:29:12.163>
labels = ['3.8', 'type-bug', '3.7']
title = 'get_type_hints fails if there are un-annotated fields in a dataclass'
updated_at = <Date 2020-03-08.13:51:23.330>
user = 'https://github.com/a-recknagel'

bugs.python.org fields:

activity = <Date 2020-03-08.13:51:23.330>
actor = 'serhiy.storchaka'
assignee = 'eric.smith'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2019-08-26.06:29:12.163>
creator = 'arne'
dependencies = []
files = []
hgrepos = []
issue_num = 37948
keywords = []
message_count = 5.0
messages = ['350488', '350973', '355559', '356829', '363662']
nosy_count = 4.0
nosy_names = ['eric.smith', 'serhiy.storchaka', 'levkivskyi', 'arne']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37948'
versions = ['Python 3.7', 'Python 3.8']

Linked PRs

@a-recknagel
Copy link
Mannequin Author

a-recknagel mannequin commented Aug 26, 2019

When declaring a dataclass with make_dataclass, it is valid to omit type information for fields. __annotations__ understands it and just adds typing.Any, but typing.get_type_hints fails with a cryptic error message:

>>> import dataclasses
>>> import typing
>>> A = dataclasses.make_dataclass('A', ['a_var'])
>>> A.__annotations__
{'a_var': 'typing.Any'}
>>> typing.get_type_hints(A)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/user/venvs/python_3.7/lib/python3.7/typing.py", line 973, in get_type_hints
    value = _eval_type(value, base_globals, localns)
  File "/user/venvs/python_3.7/lib/python3.7/typing.py", line 260, in _eval_type
    return t._evaluate(globalns, localns)
  File "/user/venvs/python_3.7/lib/python3.7/typing.py", line 464, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'typing' is not defined

Adding typing.Any explicitly is an obvious workaround:

>>> B = dataclasses.make_dataclass('B', [('a_var', typing.Any)])
>>> typing.get_type_hints(B)
{'a_var': typing.Any}

There is already a bug filed regarding datalcasses and get_type_hints which might be related: https://bugs.python.org/issue34776

@a-recknagel a-recknagel mannequin added 3.7 (EOL) end of life 3.8 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Aug 26, 2019
@ericvsmith ericvsmith self-assigned this Aug 26, 2019
@ilevkivskyi
Copy link
Member

It looks like #9518 will fix also this one.

@ericvsmith
Copy link
Member

I'm not sure what can be done with this. The problem is that the decorator doesn't know what's in the caller's namespace. The type being added is "typing.Any". If the caller doesn't import typing, then get_type_hints will fail (as demonstrated here).

The only thing I can think of is using a type that's in builtins. "object" springs to mine, but of course that's semantically incorrect.

Or, maybe I could use "dataclasses.sys.modules['typing'].Any". I don't currently import sys (I don't think), but this should be a cheap import. Then if typing.get_type_hints() is called, we know typing will have already been importing.

But what if "dataclasses" isn't in the caller's namespace? I guess if I could find some way to navigate to sys.modules from __builtins__, that would largely work, absent playing games with builtins.

@ilevkivskyi
Copy link
Member

I'm not sure what can be done with this. The problem is that the decorator doesn't know what's in the caller's namespace. The type being added is "typing.Any". If the caller doesn't import typing, then get_type_hints will fail (as demonstrated here).

IIUC the main problem is that get_type_hints() fails even if typing is imported. I would expect this to work (just repeating the original example in a more compact form):

import dataclasses
import typing
A = dataclasses.make_dataclass('A', ['a_var'])
typing.get_type_hints(A)  # This currently crashes

Interestingly, if I use a very similar call that it works:

>>> typing.get_type_hints(A, globalns=globals())
{'a_var': typing.Any}

So the core of the issue is that the globals are identified incorrectly, and indeed if I look at the generated class it looks wrong:

>>> A.__module__
'types'  # Should be '__main__'

I think we should fix the __module__ attribute of the dynamically generated dataclasses (for example the way it is done for named tuples).

Btw, #14166 may potentially fix the __module__ attribute here too.

@serhiy-storchaka
Copy link
Member

PR 14166 does not fix this issue.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@AlexWaygood
Copy link
Member

Reproduced on 3.11.07a

@AlexWaygood AlexWaygood added 3.11 only security fixes 3.10 only security fixes 3.9 only security fixes and removed 3.8 (EOL) end of life 3.7 (EOL) end of life labels Apr 14, 2022
@DavidCEllis
Copy link
Contributor

I ran into this when trying to make a quick dataclass to demo cattrs code generation.

import dataclasses
import cattrs
A = dataclasses.make_dataclass('A', ['a_var'])
a = A("a_value")
cattrs.unstructure(a)
NameError: name 'typing' is not defined. Did you forget to import 'typing'?

Initially I thought this was a cattrs bug, but on looking at the trace it uses get_type_hints so it's related to this same issue.

In 3.13.0b4, 3.12, 3.11 (and earlier) this still reproduces with:

import dataclasses
from typing import get_type_hints
A = dataclasses.make_dataclass('A', ['a_var'])
print(get_type_hints(A))

However, in 3.13.0b4, and 3.12 (but not 3.11 or earlier) if you plainly import typing this now "works" - I think because __module__ is now set when it was not in 3.11:

import dataclasses
import typing
A = dataclasses.make_dataclass('A', ['a_var'])
print(typing.get_type_hints(A))

I say "works" because it's evaluating the name 'typing' from the script file that defines the dataclass. Unlikely someone would do this, but I'd still consider this incorrect behaviour.

import dataclasses
from typing import get_type_hints
class typing:
    Any = int

A = dataclasses.make_dataclass('A', ['a_var'])
print(get_type_hints(A))
{'a_var': <class 'int'>}

It also fails if A is defined in a file that doesn't import typing and typing.get_type_hints is used from a separate file. Likewise if inspect.get_annotations(..., eval_str=True) is used you get the same error without typing needing to be imported anywhere.

Some possible solutions:

  • Use a deferred import of typing only if someone is using make_dataclass with untyped fields and put in the actual typing.Any object.
  • Track and remove the untyped fields from __annotations__ after the dataclass has been created
    • I think this is correct, but it might break code if it makes the assumption that a field existing means it's in __annotations__
  • Put a dict-like object in __annotations__ that will import and return typing.Any for untyped fields on demand.
    • This is kind of fiddly and adds a whole class just for a presumably lightly used feature

@sobolevn
Copy link
Member

Put a dict-like object in annotations that will import and return typing.Any for untyped fields on demand.

I think that this is a proper solution. This will be fixed after __annotate__ will be fully supported.

@DavidCEllis
Copy link
Contributor

If you want to avoid importing typing for the creation of such a class you still have to temporarily put something else in place for dataclasses to use in construction, and then replace it with something that will correctly evaluate afterwards. Otherwise dataclasses will trigger the imports when it inspects the annotations to create the class in the first place.

PEP649/749 could change things, but can/should this also be fixed for 3.13/3.12?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants