PEP 681: Data Class Transforms #2285

debonte · 2022-01-28T03:18:49Z

This new PEP introduces the dataclass_transform decorator, enabling dataclass-like libraries to describe their behavior to type checkers.

cc: @msfterictraut @JelleZijlstra

…pyright impl

…ynek

…ack from Hynek

From https://www.attrs.org/en/stable/api.html#attr.ib it looks like the converter is just passed the input value; the previous wording here sounded like the converter itself got passed a callable.

JelleZijlstra

Congratulations! I have a few small pieces of feedback and then we can merge the PR.

.github/CODEOWNERS

pep-9999.rst

debonte · 2022-01-28T06:38:14Z

just making sure — you know how to apply GitHub suggestions, right?

@CAM-Gerlach Thanks. I hadn't seen GitHub suggestions before and only noticed them on Jelle's comments after I addressed his changes. I'll use them going forward...

CAM-Gerlach · 2022-01-28T06:42:20Z

Great, thanks @debonte ! BTW, I'll use another of GitHub's features and mark this PR as a draft while I'm reviewing, to avoid it unintentionally getting merged while I'm halfway through.

CAM-Gerlach

Great job overall! Its a lot of little comments (if I'd known, I'd have considered making a separate PR, sorry), but mostly clarifications reST syntax changes, grammar tweaks, de-redundantizing some parts, and other little things like that, plus a few questions about parts I was unclear on as a reader. I suggest you apply the suggestions all in one batch, and then pulling that and then considering making the few larger changes after. Thanks!

pep-0681.rst

CAM-Gerlach · 2022-01-28T09:27:54Z

pep-0681.rst

+Open Issues
+===========
+
+``converter`` field descriptor parameter


Doesn't pydantic have this too—in fact, isn't it one of its central features? Might be worth mentioning here, especially given its greater modern popularity relative to attrs

Could be. Unfortunately I'm not a pydantic expert. I scanned through their docs and didn't see anything similar.

@samuelcolvin, does pydantic have a feature similar to attrs' converter field descriptor parameter?

pep-0681.rst

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

CAM-Gerlach · 2022-01-28T22:42:24Z

@AA-Turner There were multiple syntax errors (three ... in front of a directive, a stray ```suggestion, a bad link target name, etc) that the legacy build correctly failed on, and Sphinx printed ERRORs in the output, but Sphinx still finished building and returned 0, making the CI pass. Is there any way we can set Sphinx to fail on such errors, but not on warnings? I thought it did already, and I can't seem to repro it locally since they only print as WARNINGs to begin with on my end, so I'm not sure what's going on.

AA-Turner · 2022-01-28T22:43:30Z

Will have a look, thanks for the note.
A

CAM-Gerlach

Thanks! Some minor fixes to my own typos should get the build working again, my fault. And sorry again for all the comment noise; in the future I can do it as a PR to your branch or similar.

pep-0681.rst

CAM-Gerlach · 2022-01-28T22:47:09Z

(@AA-Turner We use --fail-on-warning --nitpicky to do that, but that won't work here until we fix the remaining warnings)

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

AA-Turner · 2022-01-28T23:26:02Z

Sphinx still finished building and returned 0, making the CI pass. Is there any way we can set Sphinx to fail on such errors, but not on warnings?

Fixed in #2288, you should be able to test with the bad commit bd3275f

I can't seem to repro it locally

You probably have Sphinx 4.3.*, not 4.4.0

A

CAM-Gerlach · 2022-01-29T00:10:57Z

@AA-Turner See my reply on #2288 (to avoid derailing this PR further).

CAM-Gerlach

Just a bit of final cleanup of a couple of my minor mistakes and a few style nits; otherwise LGTM! Thanks @debonte!

pep-0681.rst

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

CAM-Gerlach

LGTM, thanks @debonte ! I'll leave this up to @JelleZijlstra to merge.

JelleZijlstra

Thanks! I'll merge once CI passes.

debonte · 2022-01-31T23:37:44Z

@JelleZijlstra there are still a couple open comments on this PR. Should I address them in a separate PR?

JelleZijlstra · 2022-01-31T23:40:18Z

Sure. It looks like all of the remaining items are open-ended and may not lead to changes in the PEP. If you do decide to change something, I can merge some PRs.

debonte and others added 24 commits January 27, 2022 18:55

Convert Eric's spec to PEP format

c28c4b9

Moved motivation to abstract, added references and reference impl

42f9b2b

Add links to attrs/pedantic usage rather than describing it, link to …

d72e7ec

…pyright impl

Updates for readability and PEP style consistency

a85d639

Added motivation and rationale

eb12185

Add link to converters discussion

4b2adfa

Updates based on Eric's answers to my questions

810355a

Rearrange authors

f922582

Update 'Alternate field ordering algorithms' based on feedback from H…

ab38a98

…ynek

Second update to 'Alternate field ordering algorithms' based on feedb…

88c1e0a

…ack from Hynek

Replace __init_subclass__ with __new__

9d42083

Fix field definition typo -- copied from pyright PR 2820

748b5be

Set Jelle as sponsor

3f9e9c9

Add support for decorated classes (in addition to metaclasses)

1cb541e

Resolve open question on hierarchies with multiple transforms

4275cc4

Leave multiple decorator scenario behaviors undefined

0b78b45

Change runtime behavior to support introspection

cc57863

Introspection attribute should be an (unserialized) dict

3058c7f

Explicitly state that KW_ONLY sentinel values are supported

b256eb6

Revise author list

03835a3

fix a typo

778453f

Add entry in CODEOWNERS

bd13421

Wording change for converter (#2)

6d3e10b

From https://www.attrs.org/en/stable/api.html#attr.ib it looks like the converter is just passed the input value; the previous wording here sounded like the converter itself got passed a callable.

Remove _build directory

4150a9d

debonte requested a review from a team as a code owner January 28, 2022 03:18

the-knights-who-say-ni added the CLA signed label Jan 28, 2022

JelleZijlstra requested changes Jan 28, 2022

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

pep-9999.rst Outdated Show resolved Hide resolved

pep-9999.rst Outdated Show resolved Hide resolved

pep-9999.rst Outdated Show resolved Hide resolved

pep-9999.rst Outdated Show resolved Hide resolved

JelleZijlstra reviewed Jan 28, 2022

View reviewed changes

pep-9999.rst Outdated Show resolved Hide resolved

debonte added 2 commits January 27, 2022 22:15

Assign PEP number 681

892bbe9

Correct case of Django and EdgeDB

fb7cb8d

CAM-Gerlach marked this pull request as draft January 28, 2022 06:42

CAM-Gerlach requested changes Jan 28, 2022

View reviewed changes

CAM-Gerlach marked this pull request as ready for review January 28, 2022 09:33

debonte and others added 2 commits January 28, 2022 11:10

Apply suggestions from CAM-Gerlach's code review

720bf56

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

More suggestions from CAM-Gerlach's code review

9b49cbe

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

CAM-Gerlach reviewed Jan 28, 2022

View reviewed changes

pep-0681.rst Outdated Show resolved Hide resolved

pep-0681.rst Outdated Show resolved Hide resolved

pep-0681.rst Outdated Show resolved Hide resolved

Apply suggestions from code review

543f6e4

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

AA-Turner changed the title ~~New PEP: Data Class Transforms~~ PEP 681: Data Class Transforms Jan 28, 2022

AA-Turner mentioned this pull request Jan 28, 2022

Fail on Docutils warning #2288

Merged

Manual edits to address code review feedback

ec49246

debonte added 3 commits January 28, 2022 16:35

Link to django id field details

311c13d

DRY duplicated example descriptions

aa64756

Improve frozen language; mention ClassVar

d3af52c

CAM-Gerlach approved these changes Jan 29, 2022

View reviewed changes

pep-0681.rst Show resolved Hide resolved

pep-0681.rst Outdated Show resolved Hide resolved

pep-0681.rst Outdated Show resolved Hide resolved

Apply latest CAM-Gerlach suggestions from code review

379971f

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>

CAM-Gerlach approved these changes Jan 31, 2022

View reviewed changes

JelleZijlstra approved these changes Jan 31, 2022

View reviewed changes

JelleZijlstra merged commit 47de64f into python:main Jan 31, 2022

debonte deleted the dataclass_transforms branch March 15, 2022 01:02

erlend-aasland mentioned this pull request May 13, 2022

pep8/greppable exception messages erlend-aasland/peps#1

Closed

erlend-aasland mentioned this pull request Jun 27, 2022

pep 687/mark as accepted erlend-aasland/peps#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PEP 681: Data Class Transforms #2285

PEP 681: Data Class Transforms #2285

debonte commented Jan 28, 2022

JelleZijlstra left a comment

debonte commented Jan 28, 2022

CAM-Gerlach commented Jan 28, 2022

CAM-Gerlach left a comment

CAM-Gerlach Jan 28, 2022

debonte Jan 29, 2022

CAM-Gerlach commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

CAM-Gerlach left a comment •

edited

Loading

CAM-Gerlach commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

CAM-Gerlach commented Jan 29, 2022

CAM-Gerlach left a comment

CAM-Gerlach left a comment

JelleZijlstra left a comment

debonte commented Jan 31, 2022

JelleZijlstra commented Jan 31, 2022

PEP 681: Data Class Transforms #2285

PEP 681: Data Class Transforms #2285

Conversation

debonte commented Jan 28, 2022

JelleZijlstra left a comment

Choose a reason for hiding this comment

debonte commented Jan 28, 2022

CAM-Gerlach commented Jan 28, 2022

CAM-Gerlach left a comment

Choose a reason for hiding this comment

CAM-Gerlach Jan 28, 2022

Choose a reason for hiding this comment

debonte Jan 29, 2022

Choose a reason for hiding this comment

CAM-Gerlach commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

CAM-Gerlach left a comment • edited Loading

Choose a reason for hiding this comment

CAM-Gerlach commented Jan 28, 2022

AA-Turner commented Jan 28, 2022

CAM-Gerlach commented Jan 29, 2022

CAM-Gerlach left a comment

Choose a reason for hiding this comment

CAM-Gerlach left a comment

Choose a reason for hiding this comment

JelleZijlstra left a comment

Choose a reason for hiding this comment

debonte commented Jan 31, 2022

JelleZijlstra commented Jan 31, 2022

CAM-Gerlach left a comment •

edited

Loading