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

RecursionError when annotating a field with the same name as a field #90104

Closed
Gobot1234 mannequin opened this issue Dec 1, 2021 · 9 comments · Fixed by #100756
Closed

RecursionError when annotating a field with the same name as a field #90104

Gobot1234 mannequin opened this issue Dec 1, 2021 · 9 comments · Fixed by #100756
Assignees
Labels
3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Gobot1234
Copy link
Mannequin

Gobot1234 mannequin commented Dec 1, 2021

BPO 45946
Nosy @ericvsmith, @Gobot1234

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 2021-12-01.10:19:19.453>
labels = ['library', 'type-crash', '3.11']
title = 'RecursionError when annotating a field with the same name as a field'
updated_at = <Date 2021-12-01.14:30:55.716>
user = 'https://github.com/Gobot1234'

bugs.python.org fields:

activity = <Date 2021-12-01.14:30:55.716>
actor = 'eric.smith'
assignee = 'eric.smith'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-12-01.10:19:19.453>
creator = 'Gobot1234'
dependencies = []
files = []
hgrepos = []
issue_num = 45946
keywords = []
message_count = 1.0
messages = ['407438']
nosy_count = 2.0
nosy_names = ['eric.smith', 'Gobot1234']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue45946'
versions = ['Python 3.11']

Linked PRs

@Gobot1234
Copy link
Mannequin Author

Gobot1234 mannequin commented Dec 1, 2021

Small snippet to reproduce:

from dataclasses import dataclass, field

@dataclass
class Foo:
    bool: bool = field()

Raises

---------------------------------------------------------------------------
RecursionError                            Traceback (most recent call last)
<ipython-input-1-dce9c97e78ae> in <module>
      2 
      3 @dataclass
----> 4 class Foo:
      5     bool: bool = field()
      6 

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/dataclasses.py in dataclass(cls, init, repr, eq, order, unsafe_hash, frozen, match_args, kw_only, slots)
   1176 
   1177     # We're called as @dataclass without parens.
-> 1178     return wrap(cls)
   1179 
   1180 

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/dataclasses.py in wrap(cls)
   1167 
   1168     def wrap(cls):
-> 1169         return _process_class(cls, init, repr, eq, order, unsafe_hash,
   1170                               frozen, match_args, kw_only, slots)
   1171 

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/dataclasses.py in _process_class(cls, init, repr, eq, order, unsafe_hash, frozen, match_args, kw_only, slots)
   1085         # Create a class doc-string.
   1086         cls.__doc__ = (cls.__name__ +
-> 1087                        str(inspect.signature(cls)).replace(' -> None', ''))
   1088 
   1089     if match_args:

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/inspect.py in __str__(self)
   3200         render_kw_only_separator = True
   3201         for param in self.parameters.values():
-> 3202             formatted = str(param)
   3203 
   3204             kind = param.kind

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/inspect.py in __str__(self)
   2717         if self._annotation is not _empty:
   2718             formatted = '{}: {}'.format(formatted,
-> 2719                                        formatannotation(self._annotation))
   2720 
   2721         if self._default is not _empty:

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/inspect.py in formatannotation(annotation, base_module)
   1362             return annotation.__qualname__
   1363         return annotation.__module__+'.'+annotation.__qualname__
-> 1364     return repr(annotation)
   1365 
   1366 def formatannotationrelativeto(object):

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/dataclasses.py in __repr__(self)
    281 
    282     def __repr__(self):
--> 283         return ('Field('
    284                 f'name={self.name!r},'
    285                 f'type={self.type!r},'

... last 1 frames repeated, from the frame below ...

/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/dataclasses.py in __repr__(self)
    281 
    282     def __repr__(self):
--> 283         return ('Field('
    284                 f'name={self.name!r},'
    285                 f'type={self.type!r},'

RecursionError: maximum recursion depth exceeded while getting the repr of an object

This is due to the self.type being the field itself as the annotation is evaluated using the class namespace.

@Gobot1234 Gobot1234 mannequin added 3.11 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 1, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@neumond
Copy link

neumond commented Jul 30, 2022

Same happens in 3.10.4. My case was with date.

from dataclasses import dataclass, field
from datetime import date

@dataclass
class T:
    date: date = field()

For some reason, = field() is required to crash.

Workaround:

from dataclasses import dataclass, field
from datetime import date

date_alias = date

@dataclass
class T:
    date: date_alias = field()

@kumaraditya303 kumaraditya303 added type-bug An unexpected behavior, bug, or error 3.10 only security fixes 3.12 bugs and security fixes and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Jul 30, 2022
@StoneMoe
Copy link

StoneMoe commented Dec 29, 2022

Same here with CPython 3.10.6.
Reproduce code:

from dataclasses import dataclass, field

class Parameters:
    pass

@dataclass
class Config:
    Parameters: Parameters = field(default=None)

Crash Log:

Traceback (most recent call last):
  File "/home/deck/Documents/Bottles/test.py", line 7, in <module>
    class Config:
  File "/usr/lib/python3.10/dataclasses.py", line 1185, in dataclass
    return wrap(cls)
  File "/usr/lib/python3.10/dataclasses.py", line 1176, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
  File "/usr/lib/python3.10/dataclasses.py", line 1094, in _process_class
    str(inspect.signature(cls)).replace(' -> None', ''))
  File "/usr/lib/python3.10/inspect.py", line 3204, in __str__
    formatted = str(param)
  File "/usr/lib/python3.10/inspect.py", line 2721, in __str__
    formatannotation(self._annotation))
  File "/usr/lib/python3.10/inspect.py", line 1366, in formatannotation
    return repr(annotation)
  File "/usr/lib/python3.10/dataclasses.py", line 283, in __repr__
    return ('Field('
  File "/usr/lib/python3.10/dataclasses.py", line 283, in __repr__
    return ('Field('
  File "/usr/lib/python3.10/dataclasses.py", line 283, in __repr__
    return ('Field('
  [Previous line repeated 491 more times]
RecursionError: maximum recursion depth exceeded while getting the repr of an object

Process finished with exit code 1

@carljm
Copy link
Member

carljm commented Jan 4, 2023

This behavior is not specific to dataclasses, it is just surfaced by dataclasses because it uses annotation values in the __repr__ (most of the stdlib never pays any attention to annotation values at all.)

In general in Python, class attribute annotation name references are evaluated in the class namespace after the assignment they are a part of, which is perhaps a bit surprising. E.g. you'd probably expect that in:

class C: pass

class D:
    C = int
    x: C

that D.__annotations__["x"] is int.

But you might not expect that in:

class C: pass

class D:
    C: C = int

the same is also true, D.__annotations__["C"] is int. But that is how it works, because the relevant bytecode for the class body looks like this:

  6          12 LOAD_NAME                3 (int)
             14 STORE_NAME               4 (C)
             16 LOAD_NAME                4 (C)
             18 LOAD_NAME                5 (__annotations__)
             20 LOAD_CONST               1 ('C')
             22 STORE_SUBSCR

i.e. the new value for the name C is stored before we load the value of C to store it in __annotations__.

I'm not sure how intentional that behavior is, but it has always been that way since annotations were first introduced in Python, so I don't think it's feasible to change it now.

I also don't think it's reasonable to expect dataclasses to work around this behavior in some special way. The fact is that in Python when you shadow a name in the class namespace like this, your annotation doesn't refer to the global name, it refers to the local name in the class namespace. So the recursion error is just a result of the cycle you've created by annotating an attribute's type as... itself. Which is a very odd thing to do.

I think the best answer here is to just not do that odd thing. Add a different alias for the global name or in some way ensure that your annotation unambiguously refers to the global name, not the local name in the class namespace.

I'm closing this, since I think the behavior is expected and intended, if surprising at first, and I don't see a reasonable way to "fix" it. @ericvsmith can of course reopen if he disagrees!

For some reason, = field() is required to crash.

This is because if you have just C: C that doesn't create a new name binding at all, so C is still the global name you expect it to be. It's only if you have C: C = ... that the name C is bound to something new in the class namespace, causing this issue.

@carljm carljm closed this as not planned Won't fix, can't repro, duplicate, stale Jan 4, 2023
@carljm
Copy link
Member

carljm commented Jan 4, 2023

One possible follow-up here, not in CPython, would be to ask the type checkers (those that don't already, if some do) to bind names in the same order as the runtime, so they don't create confusion about the interpretation of this edge case. E.g. I think it would be reasonable to ask mypy to raise an error on this code saying "C is not a valid type," since that's what it does if you e.g. do C = C() in a separate prior line: https://mypy-play.net/?mypy=latest&python=3.11&gist=280d599deb242e1ffb3fa692a571807c

@carljm
Copy link
Member

carljm commented Jan 4, 2023

On second thought, while I don't think dataclasses should work around the underlying issue of which name is referred to by the annotation (and thus code like this is still broken and should be fixed to actually refer to the global name it intends to reference), it would still make sense to use the _recursive_repr decorator on Field.__repr__ and thus avoid the RecursionError here. Instead we can just get a repr with an ellipsis at the point of recursion. Attached a PR that does this.

@carljm carljm reopened this Jan 4, 2023
@StoneMoe
Copy link

StoneMoe commented Jan 5, 2023

So this is a undefined behavior, rather than a user error right?
I read through PEP-557 and PEP-526 and cannot find any specification about this. and this behavior is really counter-intuitive.

@ericvsmith
Copy link
Member

Thanks for the great description, @carljm!

I'm not sure I'd call it "undefined" behavior, but it's probably poorly documented. I haven't looked to see if there's a test for this case.

I think avoiding the RecursionError on __repr__ makes sense. I'll take a look at the PR.

@carljm
Copy link
Member

carljm commented Jan 5, 2023

It is not in the PEP, but it is documented behavior, see https://docs.python.org/3/reference/simple_stmts.html#annotated-assignment-statements

Python doesn't have a formal specification, so "undefined behavior" isn't really a meaningful term for Python the way it is for e.g. C++. The relevant questions are "what is the actual behavior?" "is it documented?" and "can we change it without breaking working code?" In this case it is documented, and in principle it would need a __future__ import to change it without potentially breaking code. In practice I'd be pretty surprised if anyone has code depending on this, but given that it is documented, the bar is pretty high for changing it if there's even a small possibility.

ericvsmith pushed a commit that referenced this issue Jan 6, 2023
…100756)

Avoid RecursionError on recursive dataclass field repr
carljm added a commit to carljm/cpython that referenced this issue Jan 6, 2023
…eld repr (pythongh-100756)

Avoid RecursionError on recursive dataclass field repr

(cherry picked from commit 0a7936a)
miss-islington pushed a commit that referenced this issue Jan 6, 2023
…pr (gh-100756) (GH-100784)

Avoid RecursionError on recursive dataclass field repr

(cherry picked from commit 0a7936a)

Automerge-Triggered-By: GH:ericvsmith
miss-islington pushed a commit that referenced this issue Jan 6, 2023
…pr (gh-100756) (GH-100785)

Avoid RecursionError on recursive dataclass field repr

(cherry picked from commit 0a7936a)

Automerge-Triggered-By: GH:ericvsmith
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants