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 in combination with Dataclasses and create_model #4949

Closed
7 of 15 tasks
mbillingr opened this issue Jan 13, 2023 · 10 comments
Closed
7 of 15 tasks

RecursionError in combination with Dataclasses and create_model #4949

mbillingr opened this issue Jan 13, 2023 · 10 comments
Labels
bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable

Comments

@mbillingr
Copy link

mbillingr commented Jan 13, 2023

Initial Checks

  • I have searched GitHub for a duplicate issue and I'm sure this is something new
  • I have searched Google & StackOverflow for a solution and couldn't find anything
  • I have read and followed the docs and still think this is a bug
  • I am confident that the issue is with pydantic (not my code, or another library in the ecosystem like FastAPI or mypy)

Description

I'm getting a RecursionError when calling pydantic.create_model with a __base__ class that contains fields which are both pydantic and vanilla dataclasses.
Most of the recursion related problems I could find were related to recursive data types (such as #1370). There is no (obvious) recursion in the data structures I use.

Traceback (most recent call last):
  File "/home/martin/.config/JetBrains/PyCharm2022.3/scratches/scratch_26.py", line 15, in <module>
    pydantic.create_model("ArbitraryName", __base__=ResponseSchema)
  File "pydantic/main.py", line 1027, in pydantic.main.create_model
  File "pydantic/main.py", line 139, in pydantic.main.ModelMetaclass.__new__
  File "pydantic/utils.py", line 695, in pydantic.utils.smart_deepcopy
  File "/home/martin/.pyenv/versions/3.11.0/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
[ whole bunch of deepcopy-related lines omitted ]
  File "/home/martin/.pyenv/versions/3.11.0/lib/python3.11/copy.py", line 272, in _reconstruct
    if hasattr(y, '__setstate__'):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^

  File "pydantic/dataclasses.py", line 255, in pydantic.dataclasses.DataclassProxy.__getattr__
    'type',

  [Previous line repeated 979 more times]
RecursionError: maximum recursion depth exceeded while calling a Python object

There is an easy workaround: Don't use both decorators.
Unfortunately, I need both because the class is defined in another module and I need to wrap it with pydantic for I/O validation.

Originally, I ran into this problem using FastAPI, but managed to reproduce it using pydantic alone. So I hope this is the correct place to ask.

Example Code

import dataclasses
import pydantic

@pydantic.dataclasses.dataclass
@dataclasses.dataclass
class ValueObj:
    name: str

class ResponseSchema(pydantic.BaseModel):
    value: ValueObj

pydantic.create_model("ArbitraryName", __base__=ResponseSchema)

Python, Pydantic & OS Version

pydantic version: 1.10.4
            pydantic compiled: True
                 install path: /home/martin/.cache/pypoetry/virtualenvs/private/lib/python3.11/site-packages/pydantic
               python version: 3.11.0 (main, Nov  7 2022, 07:51:46) [GCC 12.2.0]
                     platform: Linux-6.1.4-arch1-1-x86_64-with-glibc2.36
     optional deps. installed: ['typing-extensions']


Also reproducible with slightly older versions:

             pydantic version: 1.10.2
            pydantic compiled: True
                 install path: /home/martin/.cache/pypoetry/virtualenvs/private/lib/python3.9/site-packages/pydantic
               python version: 3.9.7 (default, Nov  7 2022, 07:48:33)  [GCC 12.2.0]
                     platform: Linux-6.1.4-arch1-1-x86_64-with-glibc2.36
     optional deps. installed: ['typing-extensions']

Affected Components

@mbillingr mbillingr added bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable labels Jan 13, 2023
@samuelcolvin
Copy link
Member

You can resolve this by removing the extra @dataclasses.dataclass.

Might have similar origin to #4907.

@mbillingr
Copy link
Author

I applied both decorators to shorten the example. In my actual code I can't remove the extra dataclass because I have the following situation:

# can't change this definition
@dataclasses.dataclass
class ValueObj:
    name: str


# IO module
ValueObj = pydantic.dataclasses.dataclass(domain.ValueObj)

P.S. Your response time is impressive!

@samuelcolvin
Copy link
Member

samuelcolvin commented Jan 13, 2023 via email

@mbillingr
Copy link
Author

I can understand that you don't feel the urge to touch the dataclass support in v1.

For me it's already a win to know that this is really a bug and not just me misusing your API. If it can't be fixed upstream I'll find a workaround. (Even though I'm not particularly fond of the idea, it should be easy to just duplicate the ValueObj definition.)

Starting Monday, I could free some time to look into this in more detail. Would you accept a pull request if I managed to find a simple fix? (I don't expect to, but you never know... :))

@samuelcolvin
Copy link
Member

Possibly, it's really up to @PrettyWood who is the "master of dataclasses".

@mbillingr
Copy link
Author

mbillingr commented Jan 16, 2023

I dug deeper and found that the recursion occurs while deep-copying a DataclassProxy.
At some point Python's copy._reconstruct function creates a new uninitialized DataclassProxy, then checks if this has a __setstate__ attribute.
DataclassProxy.__getattr__ wants to delegate the attribute access to it's __dataclass__ field, but being uninitialized, this field does not exist and looking it up invokes DataclassProxy.__getattr__ again...

I think this problem could be solved by adding a __deepcopy__ method to DataclassProxy:

class DataclassProxy:

    # ...

    def __deepcopy__(self, memo: Any) -> "DataclassProxy":
        return DataclassProxy(deepcopy(self.__dataclass__, memo))

This seems to fix my example above (I did not check if it breaks anything else, though).

@PrettyWood
Copy link
Member

Yup @mbillingr ! Seems like the right fix 👍

@mbillingr
Copy link
Author

I guess it's up to you then to decide what to do next...
I have no idea if this issue will become obsolete with V2. So if you want me to make a PR against any branch/version let me know. If you prefer to fix it yourself because a PR is too much trouble I'm fine as well.
And if you decide not to do anything my client code monkey-patch will live a long happy life :)

@samuelcolvin
Copy link
Member

I'm pretty sure this will become obsolete in V2, as the dataclass implementation will be radically simplified.

But if you'd like to create a PR again the 1.10.X-fixes branch, we'll happily review.

PrettyWood pushed a commit that referenced this issue Feb 7, 2023
* add test to reproduce #4949

* fix infinitely recursive deepcopy

* add test for shallow copy of wrapped dataclass

* fix infinitely recursive copy of wrapped dataclass

---------

Co-authored-by: Martin Billinger-Finke <2392396+mbillingr@users.noreply.github.com>
@PrettyWood
Copy link
Member

fixed by #4963

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V1 Bug related to Pydantic V1.X unconfirmed Bug not yet confirmed as valid/applicable
Projects
None yet
Development

No branches or pull requests

3 participants