Skip to content

Conversation

@sobolevn
Copy link
Member

Refs #10801
Refs #10864

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few ideas about minor tweaks.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 1, 2021

Done! @JukkaL and @JelleZijlstra, thanks a lot for the review!

(the rules are the same as in CPython):

1. All base classes (except builtin ones) must have explicit ``__slots__`` defined
2. ``__slots__`` must not include ``__dict__``. For example, ``__slots__ = ("__dict__", ...)`` is not valid.
Copy link
Collaborator

@hauntsaninja hauntsaninja Oct 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure CPython allows __slots__ to include __dict__.

If dynamic assignment of new variables is desired, then add 'dict' to the sequence of strings in the slots declaration.

So this would fall under custom rules, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very debatable whether slotted class with __dict__ is still a slotted class 🤔
Probably it can be in any category.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see where I got confused, suggested some phrasing changes :-)

(the rules are the same as in CPython):

1. All base classes (except builtin ones) must have explicit ``__slots__`` defined
2. ``__slots__`` must not include ``__dict__``. For example, ``__slots__ = ("__dict__", ...)`` is not valid.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see where I got confused, suggested some phrasing changes :-)

@sobolevn
Copy link
Member Author

sobolevn commented Oct 4, 2021

@hauntsaninja thanks, great suggestion! 👍

@hauntsaninja hauntsaninja merged commit 3ad69a0 into python:master Oct 4, 2021
@hauntsaninja
Copy link
Collaborator

Thank you for both the feature and the docs!

ilevkivskyi pushed a commit that referenced this pull request Nov 16, 2021
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.

4 participants