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

Fix recursive references when using class_schema() #189

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

noirbee
Copy link
Contributor

@noirbee noirbee commented Apr 25, 2022

Fixes #188

The @marshmallow_dataclass.dataclass decorator relies on a lazily
computed .Schema attribute/descriptor on the decorated class to solve
this issue. For class_schema() this would probably not be a valid
approach as the expectation is that its "clazz" argument would not be
modified.

Not my best work, but the @lru_cache on _internal_class_schema() made it hard to do otherwise: passing e.g. a stack of already processed classes breaks other tests which rely on the behaviour, since calls on different classes would have different stacks.

I'm also open to suggestions for a better name than seen_classes for this

The # noqa: F821 lines in the tests are due to PyCQA/pyflakes#567

Fixes lovasoa#188

The @marshmallow_dataclass.dataclass decorator relies on a lazily
computed .Schema attribute/descriptor on the decorated class to solve
this issue. For class_schema() this would probably not be a valid
approach as the expectation is that its "clazz" argument would not be
modified.
@lovasoa lovasoa merged commit 6c45757 into lovasoa:master Apr 26, 2022
@lovasoa
Copy link
Owner

lovasoa commented Apr 26, 2022

thanks

@lovasoa
Copy link
Owner

lovasoa commented Apr 26, 2022

@noirbee : would you like to be added to this repo as a collaborator ? You have been making useful pull requests, and I currently do not have a lot of time to allocate to it.

@davidecotten
Copy link

Recent changes seem to break inheritance. Is that intended?

...
from marshmallow_dataclass import dataclass


@dataclass
class ThingA:
    Schema: ClassVar[Type[Schema]]
    name: str

@dataclass
class ThingB(ThingA):
    Schema: ClassVar[Type[Schema]]
    short_name: str
>> ThingA.Schema
'ThingA'
>> ThingB.Schema
<class 'marshmallow.schema.ThingB'>

@noirbee
Copy link
Contributor Author

noirbee commented Apr 27, 2022

Recent changes seem to break inheritance. Is that intended?

Hello @davidecotten , I think this is due to interaction between the lazy_attribute_class introduced in fac0b6d and explicitly declaring a Schema ClassVar on the dataclass.

AFAICT Python's own @dataclass decorator does some introspection which triggers the descriptor's __get__() when ThingB is declared (i.e. before running ThingA.Schema), which sets its called attribute, which means it then returns a string to avoid recursion.

As a workaround, removing the ClassVar declaration fixes the problem for the base class.

I'll try to take a better look at it in favour of maybe only using the other mechanic I've introduced in this PR.

@davidecotten
Copy link

@noirbee Thank you. That makes sense and I've confirmed the workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cyclic references break when using class_schema
3 participants