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.

Sign up for GitHub

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

Jump to bottom

Exclude private attributes (PrivateAttr) from dataclass constructor #9234

Closed
1 task done
idan22moral opened this issue Apr 12, 2024 · 5 comments · Fixed by #9293
Closed
1 task done

Exclude private attributes (PrivateAttr) from dataclass constructor #9234

idan22moral opened this issue Apr 12, 2024 · 5 comments · Fixed by #9293
Labels
Change Suggested alteration to pydantic, not a new feature nor a bug

Comments

@idan22moral
Copy link
Contributor

idan22moral commented Apr 12, 2024 •
edited

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

I came across #9192 while trying to find a clue as to why private attributes (PrivateAttr) are not excluded from the (BaseModel) constructor, just like how fields (Field) are excluded when setting init=False.

The documentation states that private attributes:

are converted into a "private attribute" which is not validated or even set during calls to init, model_validate, etc.

So, in a slightly better world, when I have this class:

class Person(BaseModel):
    _secret: str = PrivateAttr()

instantiation of Person in this form (with private attributes as parameters) should not be allowed:

Person(_secret='boop')

My proposal is to simply remove the private attributes from the parameters passed to the __init__ function.

If there is a concern for backward-compatibility (assuming that users send value to private attributes in the constructor even though these values are ignored), the same concept from #8552 can be applied (adding init=False flag).
Though I think this is a not-so-good idea and the change should break compatibility in favor of better interface and less unclear behavior.

Example Code

No response

Python, Pydantic & OS Version

pydantic version: 2.7.0
        pydantic-core version: 2.18.1
          pydantic-core build: profile=release pgo=true
                 install path: C:\Users\none\AppData\Roaming\Python\Python312\site-packages\pydantic
               python version: 3.12.2 (tags/v3.12.2:6abddd9, Feb  6 2024, 21:26:36) [MSC v.1937 64 bit (AMD64)]
                     platform: Windows-10-10.0.19045-SP0
             related packages: fastapi-0.109.0 typing_extensions-4.9.0
                       commit: unknown
@idan22moral idan22moral added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Apr 12, 2024
@idan22moral idan22moral changed the title Exclude private attributes (PrivateAttr Exclude private attributes (PrivateAttr) from dataclass constructor Apr 12, 2024
@sydney-runkle
Copy link
Member

@idan22moral,

Thanks for the proposal here. I think a better route forward would probably be to emit a warning in this case, not to disallow it entirely. I think disallowing would have to wait for a major release. Feel free to open a PR adding the warning logic, if you'd like!

@sydney-runkle sydney-runkle added Change Suggested alteration to pydantic, not a new feature nor a bug and removed bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Apr 14, 2024
@idan22moral
Copy link
Contributor Author

idan22moral commented Apr 15, 2024 •
edited

Thanks for your answer, @sydney-runkle 😊

I've dug further into it, and it seems that besides the fact that pydantic allows private attributes in the BaseModel constructor, it does a good job of generating a clean constructor signature. We can see it in the __signature__ field of the model, for example:
image

To be clearer, I'm using vscode, together with the Python (v2024.4.1), Pylance (v2024.4.1) and Ruff (v2024.16.0) extensions.
What made me open this issue in the first place is the signature that Pylance showed me for the example above, which was:
image

As you can see, it's clearly much more verbose and confusing than the actual signature generated by pydantic (which is clear and correct).

So, while we can implement the warning logic as you said, I think I'll try to consult our friends over at microsoft/pylance-release first.

@idan22moral idan22moral mentioned this issue Apr 20, 2024
Merged
5 tasks
@idan22moral
Copy link
Contributor Author

@sydney-runkle After some more research, I think I've found a solution that's both backward-compatible and satisfies the type-checking needs.
It seems that the root cause of the bad signature that Pylance gives me is the fact that PrivateAttr is not considered a field specifier.

So, to let the type checkers know that PrivateAttr is a "field" that should not be part of the __init__ parameters, we need to:

  1. make PrivateAttr a field specifier.
  2. add an init parameter to the PrivateAttr function, ideally (this is a topic for discussion, of course) with type Literal[False] to never allow private attributes in __init__.

According to the definition of runtime behavior of dataclass_transform and the fact that Pydantic does not use __dataclass_transform__,
we can safely say that these changes do not affect Pydantic, but it does affect type checkers.

I made a draft PR #9293 that applies my proposed changes. I think more work regarding the added init parameter is welcome. If we decide that init should not be Literal[False] (which is a fair decision), we can drop the dedicated commit that I made for that change.

@idan22moral idan22moral mentioned this issue Apr 20, 2024
Closed
@Pyrrha
Copy link

Pyrrha commented Apr 25, 2024 •
edited

Hello,

I just encountered the same bug, but using dataclass from pydantic.dataclasses.
Should/can the same behavior be applied for this annotation as well?

Also in favor of making it throwing an exception: the usage of a signature different from the documentation provided by the IDE (and intended interface) creates inconsistency.

@sydney-runkle
Copy link
Member

sydney-runkle commented Apr 28, 2024 •
edited

Hi @idan22moral,

Thanks for looking into this with backwards compatibility in mind! Will take a look at your PR here shortly.

Curious to hear what @Viicos thinks about this approach, he seemed quite informed about all things dataclass_transform related :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Change Suggested alteration to pydantic, not a new feature nor a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Hide private attributes (PrivateAttr) from __init__ signature in type checkers idan22moral/pydantic
3 participants
@Pyrrha @idan22moral @sydney-runkle