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

Add support for __slots__ in python 3.10 #1893

Open
patrick91 opened this issue May 9, 2022 · 5 comments
Open

Add support for __slots__ in python 3.10 #1893

patrick91 opened this issue May 9, 2022 · 5 comments

Comments

@patrick91
Copy link
Member

patrick91 commented May 9, 2022

In python 3.10 you can use slots=True when applying the dataclasses decorator

see https://docs.python.org/3.10/whatsnew/3.10.html#slots

from dataclasses import dataclass

@dataclass(slots=True)
class Birthday:
    name: str
    birthday: datetime.date

we should think about whether we expose this to end users, like so:

import strawberry

@strawberry.type(slots=True)
class Birthday:
    name: str
    birthday: datetime.date

or if we enable it automatically (but we should still allow to disable it if that's the case)

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
@nrbnlulu
Copy link
Member

nrbnlulu commented Jul 10, 2022

hey @patrick91 I was trying my luck with adding __slots__ to strawberry.type and I have a question.
Does strawberry requires all of the queries / mutations / subscription to be under one huge child that is inheriting from all other?
or they are composed and does not require inheritance.
If the first is the case I'm afraid it would be impossible unless you have all unique field names and do some hacks, see this SO for more detail

@patrick91
Copy link
Member Author

@nrbnlulu we could still allow to users to enable slots manually maybe :)

field names have to be unique, so maybe we are fine? also maybe we could find a way to prevent slots from being applied to root level types (maybe types that extend other types?)

@nrbnlulu
Copy link
Member

nrbnlulu commented Jul 10, 2022

we could still allow to users to enable slots manually maybe :)

Sure, that is not the problem though hehe... but if you mentioned this, I thought to do somthing like this:

def _gte_310() -> bool:
    return sys.version_info >= (3, 10)

# inside strawberry.type we expose the slots argument
def _wrap_dataclass(cls: Type, slots: bool = _gte_310()):
    """Wrap a strawberry.type class with a dataclass and check for any issues
    before doing so"""

    # Ensure all Fields have been properly type-annotated
    _check_field_annotations(cls)

    return dataclasses.dataclass(slots=slots)(cls)

This would prevent to do pytest.mark.parameterize all over the place.
What are your thoughts?


field names have to be unique, so maybe we are fine?

Unique where? at all the schema? does fields get renamed implicitly by strawberry?

@patrick91
Copy link
Member Author

we could still allow to users to enable slots manually maybe :)

Sure, that is not the problem though hehe... but if you mentioned this, I thought to do somthing like this:

def _gte_310() -> bool:
    return sys.version_info >= (3, 10)

# inside strawberry.type we expose the slots argument
def _wrap_dataclass(cls: Type, slots: bool = _gte_310()):
    """Wrap a strawberry.type class with a dataclass and check for any issues
    before doing so"""

    # Ensure all Fields have been properly type-annotated
    _check_field_annotations(cls)

    return dataclasses.dataclass(slots=slots)(cls)

This would prevent to do pytest.mark.parameterize all over the place. What are your thoughts?

I think we can backport this feature to older versions of python :)

field names have to be unique, so maybe we are fine?

Unique where? at all the schema? does fields get renamed implicitly by strawberry?

Unique per type :) we don't do any renaming when extending, I can't remember if we error out, if that's not the case we should :)

@nrbnlulu nrbnlulu mentioned this issue Jul 10, 2022
11 tasks
@nrbnlulu
Copy link
Member

lets continue the discussion on the PR

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

Successfully merging a pull request may close this issue.

2 participants