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

PEP 681: Rename transform_descriptor_types to delete_class_attributes #2455

Closed
wants to merge 6 commits into from

Conversation

debonte
Copy link
Contributor

@debonte debonte commented Mar 22, 2022

We decided to rename transform_descriptor_types to delete_class_attributes because:

  • Descriptors only work as expected when stored in class attributes, so we needed to clarify that the class attribute was retained (not deleted) for descriptor fields.
  • Some libraries retain class attributes for all fields.

Renamed the parameter and added a new section describing the behavior in more detail.

Also removed the line about Django limitations making dataclass_transform usage impractical, since we've gotten user feedback that that's not the case. I can create a separate PR for this if preferred.

cc: @carljm @erictraut

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Code formatting on True / False:

pep-0681.rst Outdated Show resolved Hide resolved
pep-0681.rst Outdated Show resolved Hide resolved
pep-0681.rst Outdated Show resolved Hide resolved
pep-0681.rst Outdated Show resolved Hide resolved
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good apart from one nit.

Should I merge this? After we merge it, I can provide a new typing-extensions release with the new behavior.

@carljm
Copy link
Member

carljm commented Mar 23, 2022

I regret holding things up when everyone else seems satisfied, but I really don't think delete_class_attributes is an accurate description of the behavior being specified here, and I think it would be a mistake to submit the PEP with this name.

Also, after exploring the current runtime behavior of @dataclass a bit more, I have my doubts about whether this even makes sense as an added feature for the PEP.

I'll elaborate on the existing mailing list thread, since that's where the design discussion has taken place so far.

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@debonte
Copy link
Contributor Author

debonte commented Mar 28, 2022

Abandoning this PR in favor of #2477

@debonte debonte closed this Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants