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

Hide private attributes (PrivateAttr) from __init__ signature in type checkers #9293

Merged
merged 4 commits into from May 1, 2024

Conversation

idan22moral
Copy link
Contributor

@idan22moral idan22moral commented Apr 20, 2024

Change Summary

This PR:

  • adds PrivateAttr to the list of field specifiers passed to dataclass_transform by Pydantic.
  • adapts these changes in BaseModel for instance (private) attributes.

Related issue number

fix #9234.

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @hramezani

Copy link

codspeed-hq bot commented Apr 20, 2024

CodSpeed Performance Report

Merging #9293 will not alter performance

Comparing idan22moral:hide-private-attrs-in-init (d853bd4) with main (131cfa7)

Summary

✅ 13 untouched benchmarks

@idan22moral
Copy link
Contributor Author

please review :)

@Viicos
Copy link
Contributor

Viicos commented Apr 29, 2024

That's a great fix, was first worried this was unspecified behavior that pyright happened to support, but is it indeed explicitly specified by PEP 681:

init is an optional bool parameter that indicates whether the field should be included in the synthesized __init__ method. If unspecified, init defaults to True. Field specifier functions can use overloads that implicitly specify the value of init using a literal bool value type (Literal[False] or Literal[True]).

This will not handle the case where PrivateAttr isn't used:

class Model(BaseModel):
    _priv: int

Model()  # type error, ok at runtime

But we are limited by what @dataclass_transform provides.

Checked and this is also supported by mypy.


I think it's best to leave the v1 bundle untouched, @sydney-runkle what do you think?

@sydney-runkle sydney-runkle added the relnotes-fix Used for bugfixes. label Apr 30, 2024
@sydney-runkle
Copy link
Member

@idan22moral

Great work here. Love the fix.

I think it's best to leave the v1 bundle untouched, @sydney-runkle what do you think?

Agreed, let's revert those changes! Otherwise, looks ready to roll! Thanks @idan22moral for some clever and great work on this!

@idan22moral
Copy link
Contributor Author

Agreed, let's revert those changes! Otherwise, looks ready to roll! Thanks @idan22moral for some clever and great work on this!

@sydney-runkle Awesome! Thanks for the kind words 😊

I removed the v1 changes, rebased and force-pushed to preserve the clean history.
I'm marking the PR as ready for review so you can merge it.

Just making sure - are we good on the init: Literal[False] = False part?
It's not the cleanest of solutions, but I couldn't come up with something better (considering PEP 681).

@idan22moral idan22moral marked this pull request as ready for review April 30, 2024 22:03
Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Looks great!

@sydney-runkle
Copy link
Member

Just making sure - are we good on the init: Literal[False] = False part?
It's not the cleanest of solutions, but I couldn't come up with something better (considering PEP 681).

Yep, I'm ok with this. I think it's a good solution for now. We also have some time before the next minor release to change things up a bit if needed!

@sydney-runkle sydney-runkle merged commit a4a24de into pydantic:main May 1, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude private attributes (PrivateAttr) from dataclass constructor
4 participants