classification
Title: singledispatchmethod raises an error when relying on a forward declaration
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, ethan smith, glyph, gvanrossum, mental, ryansobol
Priority: normal Keywords: patch

Created on 2020-10-09 20:11 by glyph, last changed 2020-11-12 21:39 by gvanrossum.

Pull Requests
URL Status Linked Edit
PR 23216 closed mental, 2020-11-10 06:08
Messages (14)
msg378346 - (view) Author: Glyph Lefkowitz (glyph) (Python triager) Date: 2020-10-09 20:11
This example:


from __future__ import annotations
from functools import singledispatchmethod


class Comparable:
    @singledispatchmethod
    def compare(self, arg: object):
        raise NotImplementedError("what")

    @compare.register
    def _(self, arg: Comparable):
        return "somewhat similar"


print(Comparable().compare(Comparable()))


Produces this result:

  File "/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/typing.py", line 518, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'Comparable' is not defined

It seems like perhaps singledispatchmethod should defer its type evaluation to its first invocation?
msg378350 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-10-09 20:44
AFAIK the normal way of registering types (dispatcher.register(<type>)) also requires that registered type be defined at the execution type. I guess, if we are going to support such a thing, we might end up with supporting passing strings into the .register() as the initial argument of matching type to be consistent. Anyways, this would be a feature request for 3.10+, so changing the version info.
msg378353 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-10-09 21:27
This behavior (only relevant with `from __future__ import annotations`) has been around since @singledispatchmethod was introduced in 3.8, so I agree we should treat it as a feature request.

In the meantime, maybe a workaround is to move the register call out of the class? It looks a little bit ugly but probably works.

(Disclaimer: I'm not familiar with singledispatchmethod.)
msg378361 - (view) Author: Glyph Lefkowitz (glyph) (Python triager) Date: 2020-10-10 01:52
The behavior is the same with a traditional quoted forward declaration, so it’s not specific to the __future__ import; I just phrased the example that way to show how it’s going to look in the future and to illustrate how it might crop up in a way which is maximally confusing to users less familiar with the internals of type annotations.
msg379896 - (view) Author: Ryan Sobol (ryansobol) Date: 2020-10-29 22:51
It's worth pointing out that a similar error is produced for a forward-referenced return type of a registered method, but only for python3.9. For example:

from __future__ import annotations
from functools import singledispatchmethod


class Integer:
    def __init__(self, value: int):
        self.value = value

    def __str__(self) -> str:
        return str(self.value)

    @singledispatchmethod
    def add(self, other: object) -> Integer:
        raise NotImplementedError(f"Unsupported type {type(other)}")

    @add.register
    def _(self, other: int) -> "Integer":
        return Integer(self.value + other)


print(Integer(2).add(40))

This code runs without error in python3.8, and I am using this technique in code running in a production environment.

$ python3.8 --version
Python 3.8.6
$ python3.8 integer.py
42

However, this code throws a NameError in python3.9.

$ python3.9 --version
Python 3.9.0
$ python3.9 integer.py
Traceback (most recent call last):
  File "/Users/ryansobol/Downloads/integer.py", line 5, in <module>
    class Integer:
  File "/Users/ryansobol/Downloads/integer.py", line 17, in Integer
    def _(self, other: int) -> "Integer":
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/functools.py", line 909, in register
    return self.dispatcher.register(cls, func=method)
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/functools.py", line 860, in register
    argname, cls = next(iter(get_type_hints(func).items()))
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/typing.py", line 1386, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/typing.py", line 254, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/typing.py", line 497, in _evaluate
    self.__forward_value__ = _eval_type(
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/typing.py", line 254, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/typing.py", line 493, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'Integer' is not defined

I know that some may see this issue as a feature request for 3.10+. However, for me, it is a bug preventing my code from migrating to 3.9.
msg379911 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-10-30 03:34
I'm not an expert on singledispatch. It seems the get_type_hints() call is present in 3.8 as well.

Could you investigate and find a root cause? Then maybe we can fix it. (If you come up with a PR that would be very much appreciated.)
msg380797 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-11 23:50
I spent some time debugging this looking for the root cause.

I think it looks like the recursion check in ForwardRef._evaluate() fails to trigger.  At some point recursive_guard is a frozen set containing "'Integer'" (i.e. a string whose first and last character are single quotes, while self.__forward_arg__ is 'Integer' (i.e. a string that does not contain quotes).

I'm running out of time for the rest of the investigation, so feel free to confirm this and go down the rabbit hole from there...
msg380799 - (view) Author: (mental) * Date: 2020-11-12 01:10
Interesting! thanks for the hint guido I'll try to investigate further.

This sounds like a regression (to me at least), in that case should a separate issue & patch PR be opened on the bpo or should this issue be used instead?
msg380800 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-12 01:30
Keep this issue.
msg380803 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-12 04:15
FWIW here's a minimal demo:

from __future__ import annotations
from typing import get_type_hints

class C:
    def func(self, a: "C"):
        pass

    print(get_type_hints(func))

In 3.8 this prints

{'a': ForwardRef('C')}

while in 3.9 it raises NameError:

Traceback (most recent call last):
  File "C:\Users\gvanrossum\cpython\t.py", line 4, in <module>
    class C:
  File "C:\Users\gvanrossum\cpython\t.py", line 8, in C
    print(get_type_hints(func))
  File "C:\Python39\lib\typing.py", line 1386, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "C:\Python39\lib\typing.py", line 254, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "C:\Python39\lib\typing.py", line 497, in _evaluate
    self.__forward_value__ = _eval_type(
  File "C:\Python39\lib\typing.py", line 254, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "C:\Python39\lib\typing.py", line 493, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'C' is not defined
msg380832 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-12 16:01
So the biggest difference I see is that ForwardRef._evaluate() has grown a recursive_guard argument in 3.9. This makes me think that in 3.8, only one level of evaluation was happening, and in 3.8, we keep evaluating until we don't see a string or ForwardRef.

The specific examples all happen at a point where the forward ref "C" cannot be resolved at all yet (since they're happening *in the class body*).

Possibly the best way out is to treat unresolved references differently, and just return the ForwardRef to the caller -- after all this is what the example does in 3.8.
msg380843 - (view) Author: Ryan Sobol (ryansobol) Date: 2020-11-12 19:12
Does anyone know why the treatment of unresolved references was changed in 3.9?
msg380845 - (view) Author: Ryan Sobol (ryansobol) Date: 2020-11-12 19:29
Also, I'm a bit puzzled about something from the previously mentioned Integer class and its use of __future__.annotations. 

Why is it possible to declare an Integer return type for the add() method, but only possible to declare an "Integer" forward reference for the _() method?
msg380852 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2020-11-12 21:39
> Does anyone know why the treatment of unresolved references was changed in 3.9?

Probably to prepare for 3.10, where `from _future__ import annotations` is the default.

> Also, I'm a bit puzzled about something from the previously mentioned Integer class and its use of __future__.annotations. 
>
> Why is it possible to declare an Integer return type for the add() method, but only possible to declare an "Integer" forward reference for the _() method?

I don't know -- you might want to look through the source code of singledispatch. Maybe the flow through the initial decorator is different than the flow through register().
History
Date User Action Args
2020-11-12 21:39:57gvanrossumsetmessages: + msg380852
2020-11-12 19:29:50ryansobolsetmessages: + msg380845
2020-11-12 19:12:30ryansobolsetmessages: + msg380843
2020-11-12 16:01:53gvanrossumsetmessages: + msg380832
2020-11-12 04:15:28gvanrossumsetmessages: + msg380803
2020-11-12 01:30:53gvanrossumsetmessages: + msg380800
2020-11-12 01:10:27mentalsetmessages: + msg380799
2020-11-11 23:50:40gvanrossumsetmessages: + msg380797
2020-11-10 06:08:22mentalsetkeywords: + patch
nosy: + mental

pull_requests: + pull_request22113
stage: needs patch -> patch review
2020-10-30 03:34:17gvanrossumsetmessages: + msg379911
versions: + Python 3.9
2020-10-29 22:51:46ryansobolsetnosy: + ryansobol
messages: + msg379896
2020-10-10 01:52:04glyphsetmessages: + msg378361
2020-10-09 21:27:20gvanrossumsetnosy: + ethan smith
messages: + msg378353
2020-10-09 20:44:24BTaskayasetnosy: + gvanrossum

messages: + msg378350
versions: + Python 3.10, - Python 3.8
2020-10-09 20:38:25BTaskayasetnosy: + BTaskaya
2020-10-09 20:11:43glyphcreate