Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

bpo-46382 dataclass(slots=True) now takes inherited slots into account#31980

Merged
ericvsmith merged 6 commits intopython:mainpython/cpython:mainfrom
ariebovenberg:fix-issue-46382ariebovenberg/cpython:fix-issue-46382Copy head branch name to clipboard
Mar 19, 2022
Merged

bpo-46382 dataclass(slots=True) now takes inherited slots into account#31980
ericvsmith merged 6 commits intopython:mainpython/cpython:mainfrom
ariebovenberg:fix-issue-46382ariebovenberg/cpython:fix-issue-46382Copy head branch name to clipboard

Conversation

@ariebovenberg
Copy link
Contributor

@ariebovenberg ariebovenberg commented Mar 18, 2022 •
edited by bedevere-bot
Loading

see https://bugs.python.org/issue46382

Questions

  • Would we want to backport? I wouldn't recommend as this change may technically be breaking for some rare cases
  • I assume we would want to include this fix in the dataclasses add_slots function?

https://bugs.python.org/issue46382

@ericvsmith
Copy link
Member

ericvsmith commented Mar 18, 2022

I don't think we want to backport this, for the reason you mention.

If you mean add_slots() in the ericvsmith/dataclasses repo, then I wouldn't oppose it being added there, although I consider that package basically abandoned at this point. That said, I myself use it for a client who's stuck on 3.6, although I don't use __slots__.

ericvsmith
ericvsmith requested changes Mar 18, 2022
View reviewed changes
Copy link
Member

@ericvsmith ericvsmith left a comment

Choose a reason for hiding this comment

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

A few nits, but otherwise looks good. Kudos on the match statement!

Doc/library/dataclasses.rst Outdated Show resolved Hide resolved
Doc/library/dataclasses.rst Show resolved Hide resolved
Lib/test/test_dataclasses.py Show resolved Hide resolved
Lib/test/test_dataclasses.py Outdated Show resolved Hide resolved
Lib/dataclasses.py Show resolved Hide resolved
Lib/dataclasses.py Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Mar 18, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ariebovenberg
Copy link
Contributor Author

ariebovenberg commented Mar 18, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Mar 18, 2022

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from ericvsmith March 18, 2022 17:52
ericvsmith
ericvsmith reviewed Mar 18, 2022
View reviewed changes
Lib/test/test_dataclasses.py Show resolved Hide resolved
@ariebovenberg
Copy link
Contributor Author

ariebovenberg commented Mar 19, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Mar 19, 2022

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from ericvsmith March 19, 2022 07:44
AlexWaygood
AlexWaygood reviewed Mar 19, 2022
View reviewed changes
Lib/test/test_dataclasses.py Outdated Show resolved Hide resolved
ariebovenberg and others added 2 commits March 19, 2022 11:34
@ariebovenberg @AlexWaygood
use specific unittest assert for membership test
5d23ba1 
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@ariebovenberg
fix ref in news item
e11bd82
ericvsmith
ericvsmith reviewed Mar 19, 2022
View reviewed changes
Misc/NEWS.d/next/Library/2022-03-18-17-25-57.bpo-46382.zQUJ66.rst Outdated Show resolved Hide resolved
@ariebovenberg
Copy link
Contributor Author

ariebovenberg commented Mar 19, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Mar 19, 2022

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from ericvsmith March 19, 2022 14:03
@ericvsmith ericvsmith merged commit 82e9b0b into python:main Mar 19, 2022
@ariebovenberg ariebovenberg deleted the fix-issue-46382 branch March 19, 2022 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@ericvsmith ericvsmith Awaiting requested review from ericvsmith ericvsmith is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ariebovenberg @ericvsmith @bedevere-bot @AlexWaygood @the-knights-who-say-ni
Morty Proxy This is a proxified and sanitized view of the page, visit original site.