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

gh-90562: Mention slots pitfall in dataclass docs #107391

Merged
merged 10 commits into from
May 21, 2024
Merged

Conversation

thejcannon
Copy link
Contributor

@thejcannon thejcannon commented Jul 28, 2023

Doesn't fix the issue, but does call it out so developers know that the issue doesn't lie with them (per-se) and gives them more info on where to go.

Will need to be backported to 3.10 and 3.11.


📚 Documentation preview 📚: https://cpython-previews--107391.org.readthedocs.build/

@ericvsmith
Copy link
Member

I can't say I've seen something like this documented before. I'd like to get some more opinions on it.

@thejcannon
Copy link
Contributor Author

Yeah. If the exception was more clear, maybe it wouldn't be warranted. But it took me longer than I want to admit to put two-and-two together. And then, because I'm super nosy I was able to find the linked issue.

So I feel for the little guy who has to piece it all together. (Speaking of, I should edit to suggest the workaround, the "old-style" super call)

@AA-Turner AA-Turner added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Jul 28, 2023
is raised. Calling no-arg ``super()`` in dataclasses using ``slots=True`` will result in
the following Exception being raised:
``TypeError: super(type, obj): obj must be an instance or subtype of type``.
The two-arg ``super()`` is a valid workaround. See :issue:`46404` for full details.
Copy link
Member

Choose a reason for hiding this comment

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

We should try to capture everything relevant in the documentation -- what is relevant to the reader from the linked issue that isn't captured in your summary?

A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think these two points are it. What to look for and how to workaround.

@carljm
Copy link
Member

carljm commented Jul 28, 2023

I feel like early in my CPython contribution I remember someone saying "we don't document bugs, we fix them."

But I'm also sympathetic to the point of view that this is a significant limitation that people are likely to hit and with an extremely confusing symptom, and there's no clear fix for it on the horizon, and it would be kinder to our users to surface it in the docs.

So while it bugs me slightly, on the whole I think I'm in favor of this. I'll leave the final say to our documentation experts (and @ericvsmith as dataclasses expert.)

Copy link
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

This seems sensible to me. It took me a while to find out what was going on when I encountered this issue.

Doc/library/dataclasses.rst Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Apr 11, 2024

(This now has a merge conflict.)

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024
Doc/library/dataclasses.rst Outdated Show resolved Hide resolved
@hugovk hugovk enabled auto-merge (squash) May 21, 2024 12:06
@hugovk hugovk merged commit e3ed574 into python:main May 21, 2024
25 checks passed
@miss-islington-app
Copy link

Thanks @thejcannon for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 21, 2024
)

(cherry picked from commit e3ed574)

Co-authored-by: Josh Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 21, 2024
)

(cherry picked from commit e3ed574)

Co-authored-by: Josh Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 21, 2024

GH-119350 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 21, 2024
@bedevere-app
Copy link

bedevere-app bot commented May 21, 2024

GH-119351 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label May 21, 2024
@thejcannon thejcannon deleted the patch-1 branch May 21, 2024 19:43
@thejcannon
Copy link
Contributor Author

Huzzah! 🎉

hugovk added a commit that referenced this pull request May 23, 2024
…119350)

Co-authored-by: Josh Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
hugovk added a commit that referenced this pull request May 23, 2024
…119351)

Co-authored-by: Josh Cannon <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@e-gebes
Copy link

e-gebes commented Jun 19, 2024

Could you backport this also to the 3.10 docs? I ran into the issue, and since I need to use 3.10 atm I read the 3.10 docs.

@hugovk
Copy link
Member

hugovk commented Jun 19, 2024

Sorry, 3.10 and 3.11 are only allowed to have security fixes: https://devguide.python.org/versions/

estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.