GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-90562: Improve zero argument support for `super()` in dataclasses when `slots=True` by Bobronium · Pull Request #124692 · python/cpython

Conversation

@Bobronium

@Bobronium Bobronium commented Sep 27, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Original PR: gh-90562: Support zero argument super with dataclasses when slots=True #124455

Added two tests for two cases that weren't covered in original PR:

  • Using custom descriptors
  • Wrapping function before making it a property
  • Issue: RFE: @dataclasses.dataclass(slots=True) doesn't support methods using zero-arg super() #90562

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Bobronium
Bobronium requested a review from ericvsmith as a code owner September 27, 2024 18:05
@ghost

ghost commented Sep 27, 2024 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting review label Sep 27, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Sep 27, 2024
RFE: @dataclasses.dataclass(slots=True) doesn't support methods using zero-arg super() #90562
Closed
@sobolevn
sobolevn requested a review from carljm September 27, 2024 18:10
@Bobronium
Bobronium force-pushed the issue-90562-dataclasses-zero-argument-super-fix branch from 2e0ca01 to 3b65acf Compare September 27, 2024 18:10
ericvsmith
ericvsmith reviewed Sep 27, 2024
View reviewed changes
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/dataclasses.py Outdated
_seen.add(id(obj))

_depth += 1
if _depth > 2:

@ericvsmith ericvsmith Sep 27, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why are depth 1 and 2 special? What is this test (and _depth in general) trying to accomplish? Please add comments explaining what's going on here, it's not at all clear from the code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Bobronium reacted with thumbs up emoji Bobronium reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

@Bobronium Bobronium Sep 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added an explanation and few test cases for this behaviour. Caught two bugs in the process \o/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/dataclasses.py Outdated
):
# we don't want to inspect custom descriptors just yet
# there's still a chance we'll encounter a pure function
# or a property

@ericvsmith ericvsmith Sep 27, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add a comment explaining why encountering a pure function or property means we don't have to inspect custom descriptors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Bobronium Bobronium Sep 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hmm, there's already an explanation above:

cpython/Lib/dataclasses.py

Lines 1336 to 1339 in 3b65acf

# original class. We can break out of this loop as soon as we
# make an update, since all closures for a class will share a
# given cell. First we try to find a pure function/properties,
# and then fallback to inspecting custom descriptors.

Would you like me to add clarification for this case specifically?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@ericvsmith ericvsmith Sep 27, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah, you're right. Maybe change the comment to "... fallback to inspecting custom descriptors if no pure function or property is found".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Bobronium Bobronium Sep 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Makes sense. I've added clarification in both places to be extra clear.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
ericvsmith
ericvsmith reviewed Sep 27, 2024
View reviewed changes
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sobolevn
sobolevn reviewed Sep 28, 2024
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Great find, thanks a lot for your work! 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Bobronium reacted with heart emoji
All reactions
  • ❤️ 1 reaction
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/dataclasses.py Outdated
return None

for attr in dir(obj):
value = getattr(obj, attr, None)

@sobolevn sobolevn Sep 28, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This case is always complex, since getattr can evaluate properties and other objects. There are two ways of hanling this case:

  1. Allow all exceptions to raise
  2. Hide all exceptions from user here

I am not quite sure which one is better here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Bobronium Bobronium Sep 28, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for bringing this to my attention!

After some hacking, I've decided to go with the third option: removing any possibility of user defined code execution.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sobolevn reacted with eyes emoji
All reactions
  • 👀 1 reaction
Comment thread Lib/test/test_dataclasses/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
carljm
carljm reviewed Oct 1, 2024
View reviewed changes

@carljm carljm left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks OK to me; I'll let @ericvsmith give it final approval/merge.

I don't love that we've opened the door to an infinite fractal of complexity here; at some point it would probably be better to bite the bullet and introduce a __classcell__ attribute on classes that allows reliably finding this cell in O(1) time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/dataclasses.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ericvsmith

ericvsmith commented Oct 2, 2024

Copy link
Copy Markdown
Member

I agree that the complexity is unfortunate. __classcell__ would be a good change. In the meantime, could inspect.getattr_static or inspect.getmembers_static be used instead of _safe_get_attributes?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Bobronium

Bobronium commented Oct 5, 2024

Copy link
Copy Markdown
Contributor Author

In the meantime, could inspect.getattr_static or inspect.getmembers_static be used instead of _safe_get_attributes?

Good idea. I've changed the implementation to use inspect.getmembers_static() instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Bobronium

Bobronium commented Nov 3, 2024

Copy link
Copy Markdown
Contributor Author

@carljm, friendly ping :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@carljm

carljm commented Nov 4, 2024

Copy link
Copy Markdown
Member

As I mentioned above, I would rather have @ericvsmith approve/merge this, since it was his review comments you addressed.
Bobronium and simonsan reacted with thumbs up emoji Bobronium reacted with heart emoji
All reactions
  • 👍 2 reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericvsmith

ericvsmith commented Nov 4, 2024

Copy link
Copy Markdown
Member

I've been busy, but it's on my list of things to do.
Bobronium reacted with thumbs up emoji Bobronium reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ZetaTwo ZetaTwo mentioned this pull request Dec 17, 2024
Use dataclass slots to decrease memory usage SiegeEngineers/genieutils-py#4
Merged
@nthykier nthykier mentioned this pull request Sep 21, 2025
Breaks super call with @dataclasses.dataclass(slots=True) asottile/pyupgrade#1032
Closed
@danielhollas danielhollas mentioned this pull request Feb 2, 2026
gh-137855: Lazy import inspect module in dataclasses #144387
Merged
@github-actions

github-actions Bot commented Apr 18, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 18, 2026
Bobronium added 10 commits October 20, 2026 23:14
@Bobronium @ambv
Improve zero argument support for super() in dataclasses
475794b
@Bobronium @ambv
Fix NEWS.d formatting
75e769d
@Bobronium @ambv
Add explanation for _depth use and tests
3c09e3a
@Bobronium @ambv
Fix incorrect variable reference
d53d538
@Bobronium @ambv
Correctly unwrap descriptor function
a9c4319
@Bobronium @ambv
Add test with using partial as custom wrapper
26e13e7
@Bobronium @ambv
Rephrase comment explaining _depth argument
3a08b0e
@Bobronium @ambv
Add clarification for function search priority
2e9588d
@Bobronium @ambv
Ues consistent grammar throughout comments
fb4bc9f
@Bobronium @ambv
Fix breaking out from outer loop when handling property
bdaeb1f
Bobronium and others added 7 commits October 20, 2026 23:14
@Bobronium @ambv
Use generator expression with any() instead of classic loop
8f6140a
@Bobronium @sobolevn @ambv
Apply suggestions from code review
13c2dc7 
Co-authored-by: sobolevn <[email protected]>
@Bobronium @ambv
Remove leading underscores from seen and depth args
9d3d49f
@Bobronium @ambv
Prevent user-defined code execution during attribute scanning
d095864
@Bobronium @ambv
Use inspect.getattr_static instead of _safe_get_attributes
6848c1b 
This uses builtins.dir, which will trigger custom __getattibute__ if any, and will trigger __get__ on __dict__ descriptor.
@Bobronium @ambv
Put back the comment in _update_func_cell_for__class__
1d443e7
@Bobronium @ambv
Rely on inspect.getmembers_static()
5a7037b
@ambv
ambv force-pushed the issue-90562-dataclasses-zero-argument-super-fix branch from 5bfea4d to 5a7037b Compare October 20, 2026 21:17
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 20, 2026
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm left review comments
@ericvsmith ericvsmith ericvsmith left review comments
@sobolevn sobolevn sobolevn left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Bobronium @ericvsmith @carljm @sobolevn
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.