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

Can Strawberry created dataclasses be frozen by default? #3396

Open
1 of 3 tasks
kkom opened this issue Feb 25, 2024 · 3 comments
Open
1 of 3 tasks

Can Strawberry created dataclasses be frozen by default? #3396

kkom opened this issue Feb 25, 2024 · 3 comments

Comments

@kkom
Copy link

kkom commented Feb 25, 2024

Feature Request Type

  • Core functionality
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

Need

Since 1.1.328 Pyright type checker has become more strict w.r.t. to detecting incompatible variable overrides.

Specifically code like this:

class MyParent:
  my_field: int | str

class MyChild(MyParent):
  my_field: int  # `reportIncompatibleVariableOverride` detected here

Now triggers a type error, because of this failure scenario:

def mutate_parent(p: MyParent) -> None:
  p.my_field = "foo"

def process_child(c: MyChild) -> None:
  mutate_parent(c)

  c.my_field  # the typechecker believes the type is `int`, but the actual value at runtime is `"foo"` and the type is `str`

However, if MyParent was a frozen dataclass everything would be fine – because Pyright would be convinced that the value isn't mutated when processing the value using the wider parent type.

A scenario where this is useful is interaction between GraphQL types and interfaces they implement. It's common for a GraphQL interface to be wider than the specific type that implements this. In order to represent this through Python inheritance, any such fields needs to be implemented as a Python method, rather than instance/class variables. This is because the return types of the former are interpreted as immutable by Pyright.

Therefore, I suggest that dataclasses created to represent Strawberry GraphQL types are frozen dataclasses. I don't think there is any real use case for mutating field values in an already constructed Python object.

Implementation

I'm sure the solution is somewhere in PEP 681, especially because there are plenty of occurrences of the term frozen in its text. However, it's not immediately clear to me how this should be done – I've never written a dataclass transform myself.

Happy to dig into it a little bit myself, if nobody else has an immediate idea how to do it though. :)

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@kkom
Copy link
Author

kkom commented Feb 25, 2024

Okay, I think we need to add frozen_default=True in places like this:

order_default=True, kw_only_default=True, field_specifiers=(field, StrawberryField)

order_default=True, kw_only_default=True, field_specifiers=(field, StrawberryField)

See: python/cpython#99957 and the updated spec here: https://typing.readthedocs.io/en/latest/spec/dataclasses.html#dataclass-transform-parameters

@kkom
Copy link
Author

kkom commented Feb 25, 2024

I've created #3397 for it – I suggest we continue the discussion there, as it's closer to code :)

@kkom
Copy link
Author

kkom commented Feb 26, 2024

One step closer to drawing the issue I created to an unfortunately unsatisfying end. The performance hit of this is pretty substantial, see more: #3397 (comment)

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

No branches or pull requests

1 participant