Skip to content

Move dataclass kw_only fields to the end of the signature - #19018

Merged
JukkaL merged 5 commits into
python:masterfrom
sterliakov:bugfix/gh-19017-dataclass-fields-kwonly-order
Jun 5, 2025
Merged

Move dataclass kw_only fields to the end of the signature#19018
JukkaL merged 5 commits into
python:masterfrom
sterliakov:bugfix/gh-19017-dataclass-fields-kwonly-order

Conversation

@sterliakov

@sterliakovsterliakov commented May 3, 2025

Copy link
Copy Markdown
Collaborator

Fixes#19017. Fixes#17731.

This is a rather naive change: python does that at runtime. kw_only args can be in any order, and non-kwonly args should remain sorted as-is (stable sort). I don't understand why this was only done in presence of a parent dataclass - AFAIC kwonly fields work that way since kw_only was introduced in py3.10.

The test I changed was invalid and asserted a false positive to the best of my knowledge.

@github-actions

This comment has been minimized.

@sterliakov
sterliakov marked this pull request as ready for review May 3, 2025 16:33
@sterliakov
sterliakov requested a review from A5rocksMay 3, 2025 16:34

@A5rocksA5rocks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I find the found_dataclass_supertype logic confusing -- why was that there?

Because this makes sense.

@sterliakov

Copy link
Copy Markdown
CollaboratorAuthor

Given that we had a test asserting such behavior, I suspect #13539 was just a rushed implementation with some cases mistreated or misunderstood. I can't find any explanation to sorting the attributes only if there was a parent class - perhaps the author assumed that kw_only fields must be in trailing positions only and consequently skipped sorting as a perf optimization.

@NeilGirdhar

Copy link
Copy Markdown
Contributor

Will this PR also fix #17564?

@sterliakov

sterliakov commented May 9, 2025

Copy link
Copy Markdown
CollaboratorAuthor

@NeilGirdhar

No, it won't, because x is kn_only there and y is not, it's still sorted the wrong way... So we can't sort attributes once and forever, they may have different order in __init__ and __post_init__. The generated signatures in your example should be

# Follows the "jumping" logic of kw_onlydef__init__(self, y: str, *, x: int): ...
# Definition order, reverse MRO order, no keywordsdef__post_init__(self, x: int, y: str, /): ...

(a bit more difficult with defaults, but similar still)

Wait, I'm still confused. I'll look deeper into runtime behaviour of InitVar and inheritance tonight. No, that's right, InitVars just aren't affected by their kn_only-ness, since __post_init__ is posonly. That makes sense.

@NeilGirdhar

Copy link
Copy Markdown
Contributor

Thanks for explaining!

@sterliakov

Copy link
Copy Markdown
CollaboratorAuthor

#17564 is an easy fix, let's merge this and I will open a follow-up PR.

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for freeto 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.

kw_only dataclass fields confuse mypy on argument order Attributes without a default cannot follow attributes with one

4 participants

@sterliakov@NeilGirdhar@A5rocks@JukkaL