◐ Shell
reader mode source ↗
Skip to content

gh-90562: Improve zero argument support for super() in dataclasses when slots=True#124692

Open
Bobronium wants to merge 18 commits into
python:mainfrom
Bobronium:issue-90562-dataclasses-zero-argument-super-fix
Open

gh-90562: Improve zero argument support for super() in dataclasses when slots=True#124692
Bobronium wants to merge 18 commits into
python:mainfrom
Bobronium:issue-90562-dataclasses-zero-argument-super-fix

Conversation

@Bobronium

@Bobronium Bobronium commented Sep 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

  • Using custom descriptors
  • Wrapping function before making it a property

@ghost

ghost commented Sep 27, 2024 •
edited by ghost
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app Bot mentioned this pull request Sep 27, 2024
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
sobolevn
sobolevn reviewed Sep 28, 2024
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Great find, thanks a lot for your work! 👍

carljm
carljm reviewed Oct 1, 2024
View reviewed changes

@carljm carljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

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.

@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?

@Bobronium
Use inspect.getattr_static instead of _safe_get_attributes …
c68b4cc 
This uses builtins.dir, which will trigger custom __getattibute__ if any, and will trigger __get__ on __dict__ descriptor.
@Bobronium
Put back the comment in _update_func_cell_for__class__
7d679f2
@Bobronium
Rely on inspect.getmembers_static()
5bfea4d
@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.

@Bobronium

Bobronium commented Nov 3, 2024

Copy link
Copy Markdown
Contributor Author

@carljm, friendly ping :)

@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.

@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.

@ZetaTwo ZetaTwo mentioned this pull request Dec 17, 2024
Merged
@nthykier nthykier mentioned this pull request Sep 21, 2025
Closed
@danielhollas danielhollas mentioned this pull request Feb 2, 2026
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.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm

@ericvsmith ericvsmith ericvsmith left review comments

@sobolevn sobolevn sobolevn left review comments

Assignees

No one assigned

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Bobronium @ericvsmith @carljm @sobolevn