classification
Title: Cross-module dataclass inheritance breaks get_type_hints
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: AlexWaygood, Jelle Zijlstra, aidan.b.clark, eric.smith, gvanrossum, kj, slebedev, sobolevn
Priority: normal Keywords: patch

Created on 2021-10-19 14:37 by aidan.b.clark, last changed 2022-01-22 08:37 by AlexWaygood.

Pull Requests
URL Status Linked Edit
PR 29158 open sobolevn, 2021-10-22 12:15
Messages (13)
msg404307 - (view) Author: Aidan Clark (aidan.b.clark) Date: 2021-10-19 14:37
[I believe this is fundamentally a dataclass version of https://bugs.python.org/issue41249]

When using `from __future__ import annotations`, calling get_type_hints on the constructor of a dataclass B which inherits from a dataclass A defined in another module will error if dataclass A has type hints which are not imported in the module where dataclass B is defined.

This is best shown by example, if you have foo.py:

```
from __future__ import annotations

import collections
import dataclasses

@dataclasses.dataclass
class A:
  x: collections.OrderedDict
```

and then in bar.py:

```
from __future__ import annotations

import foo
import dataclasses
import typing

@dataclasses.dataclass
class B(foo.A):
  pass

typing.get_type_hints(B)
```

the final line will raise "NameError: name 'collections' is not defined".


This code will not error if you do either of the following:
  * add `import collections` to bar.py.
  * remove the __future__ annotations import from both files.


I am not confident enough on the internals of dataclass to suggest a fix, but potentially a similar approach to that which solved the TypedDict equivalent https://bugs.python.org/issue41249 would work?
msg404312 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2021-10-19 16:01
I can't reproduce this on Python 3.8.3, 3.9.6, 3.10.0 or 3.11.0a1+. Which versions of Python have you tried this on? (I'm able to reproduce the `typing.TypedDict` bug on Python 3.8, since the fix was only backported to 3.9, but not this.)
msg404313 - (view) Author: Sergei Lebedev (slebedev) Date: 2021-10-19 16:14
I think the example has a minor typo. 

The crash is reproducible on 3.9 if the last line in bar.py is

    typing.get_type_hints(B.__init__)

instead of

    typing.get_type_hints(B)
msg404314 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2021-10-19 16:31
Thanks @Sergei. With that alteration, I have reproduced this on Python 3.9, 3.10 and 3.11.
msg404321 - (view) Author: Aidan Clark (aidan.b.clark) Date: 2021-10-19 17:38
Ah yes, I completely apologize about that typo, the __init__ is definitely needed (serves me right for trying to clean up a repo before posting it).

One other comment to make, perhaps obvious; manually passing a namespace to get_type_hints' `globalns` which contains `collections` does indeed solve the problem (in case anyone else sees this and is blocked). Of course determining the right values to pass is in general difficult for a user (and shouldn't be necessary).
msg404740 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-10-22 10:01
I had some time to debug this. It happens because we treat classes and functions differently.

The difference between `get_type_hints(B)` and `get_type_hints(B.__init__)` is that we use different `global` contexts there:
- For `B` we use `__mro__` entries and extract `globals` and `locals` from there: https://github.com/python/cpython/blob/86dfb55d2e091cf633dbd7aabcd49d96fb1f9d81/Lib/typing.py#L1792-L1796
- For `B.__init__` we use simplier logic https://github.com/python/cpython/blob/86dfb55d2e091cf633dbd7aabcd49d96fb1f9d81/Lib/typing.py#L1822-L1828 It does not know anything about `__mro__` and super contexts

Funny thing, this problem goes away if we remove `@dataclass` decorator and convert our examples into regular classes:

```
# a.py
from __future__ import annotations

import collections


class A:
  x: collections.OrderedDict
  def __init__(self, x: collections.OrderedDict) -> None:
      ...
```

and

```
# b.py
from __future__ import annotations

import a
import typing

class B(a.A):
  pass

print(typing.get_type_hints(B))  
# {'x': <class 'collections.OrderedDict'>}
print(typing.get_type_hints(B.__init__))
# {'x': <class 'collections.OrderedDict'>, 'return': <class 'NoneType'>}
```

I am going to try to solve this with something really simple (if no one else is working on it).
msg404752 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2021-10-22 11:09
@Nikita, I had a go at writing some more rigorous tests regarding this issue. I found the same thing you did -- the issue seems:

* Isolated to dataclasses specifically (doesn't occur with TypedDicts, standard classes or NamedTuples)
* Isolated to the __init__ method of dataclasses
* Only occurs when you *subclass* a dataclass defined in another module.

My tests are in these two files on my cpython fork:

* https://github.com/AlexWaygood/cpython/blob/forward-annotations-bpo-45524/Lib/test/test_future_annotations.py
* https://github.com/AlexWaygood/cpython/blob/forward-annotations-bpo-45524/Lib/test/_typing_imports_helper.py

(I'm not proposing adding two new files to the cpython test suite -- just put the tests in new files so that I could isolate the new tests from the rest of the test suite and understand the problem better.)
msg404754 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2021-10-22 11:22
If you try running my test script with the `from __future__ import annotations` line at the top commented out, however, the error is the same. (`from __future__ import annotations` is obviously still there in the module that's being imported by the test script.)
msg404812 - (view) Author: Sergei Lebedev (slebedev) Date: 2021-10-22 18:48
Is it worth also addressing the case where a @dataclass/typing.TypeDict class is defined within a function?

```                                                                                                  
from __future__ import annotations

import typing
from dataclasses import dataclass


def make_A():
  import collections

  @dataclass
  class A:
    x: collections.defaultdict

  return A


A = make_A()

@dataclass
class B(A):
  y: int

# NameError: name 'collections' is not defined
print(typing.get_type_hints(B.__init__))
```
msg404822 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2021-10-22 20:30
@Sergei, I believe that's a much larger issue, and one that's quite difficult to solve. It's one of the principal reasons why `from __future__ import annotations` behaviour wasn't made the default in Python 3.10, as was originally the plan. (This behaviour doesn't play nicely with libraries such as pydantic that analyse annotations at runtime.) I think it's probably beyond the scope of this BPO issue :)

https://mail.python.org/archives/list/python-dev@python.org/thread/CLVXXPQ2T2LQ5MP2Y53VVQFCXYWQJHKZ/
msg404823 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-10-22 20:38
I completelly agree that the exable above is out of scope. Because it requires `locals()` to be modified. While this issue is focused on `globals()`.
msg404907 - (view) Author: Sergei Lebedev (slebedev) Date: 2021-10-23 19:46
Is it worth filing a separate issue for locals()? 

In my experience local classes are less common than cross-module inheritance, so I suspect that the chances of someone accidentally hitting lack of locals() forwarding are quite low. However, given how confusing the error message is, it might be worth having an issue for that. Wdyt?
msg404918 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-10-24 08:35
In my opinion, it is never a bad thing to create a new issue :)

сб, 23 окт. 2021 г. в 22:46, Sergei Lebedev <report@bugs.python.org>:

>
> Sergei Lebedev <slebedev@google.com> added the comment:
>
> Is it worth filing a separate issue for locals()?
>
> In my experience local classes are less common than cross-module
> inheritance, so I suspect that the chances of someone accidentally hitting
> lack of locals() forwarding are quite low. However, given how confusing the
> error message is, it might be worth having an issue for that. Wdyt?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue45524>
> _______________________________________
>
History
Date User Action Args
2022-01-22 08:37:13AlexWaygoodsetnosy: + Jelle Zijlstra
2021-11-30 00:18:33eric.smithsetassignee: eric.smith
2021-10-24 08:35:06sobolevnsetmessages: + msg404918
2021-10-23 19:46:26slebedevsetmessages: + msg404907
2021-10-22 20:38:46sobolevnsetmessages: + msg404823
2021-10-22 20:30:56AlexWaygoodsetmessages: + msg404822
2021-10-22 18:48:53slebedevsetmessages: + msg404812
2021-10-22 14:46:38AlexWaygoodsetnosy: + gvanrossum, kj
2021-10-22 12:15:34sobolevnsetkeywords: + patch
stage: patch review
pull_requests: + pull_request27433
2021-10-22 11:22:12AlexWaygoodsetmessages: + msg404754
2021-10-22 11:09:29AlexWaygoodsetmessages: + msg404752
2021-10-22 10:01:36sobolevnsetnosy: + sobolevn
messages: + msg404740
2021-10-19 17:38:39aidan.b.clarksetmessages: + msg404321
2021-10-19 16:31:28AlexWaygoodsetversions: + Python 3.10, Python 3.11, - Python 3.6
2021-10-19 16:31:14AlexWaygoodsetmessages: + msg404314
2021-10-19 16:23:02xtreaksetnosy: + eric.smith
2021-10-19 16:14:29slebedevsetmessages: + msg404313
2021-10-19 16:01:48AlexWaygoodsetnosy: + AlexWaygood
messages: + msg404312
2021-10-19 14:37:47aidan.b.clarkcreate