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
Added initvars support to post_init_post_parse #748
Conversation
Codecov Report
@@ Coverage Diff @@
## master #748 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 2723 2723
Branches 535 536 +1
=====================================
Hits 2723 2723 |
Is there an application you have in mind where you would benefit from having access to the initvars in If that is typical, it seems like requiring the initvars be accepted as arguments to the But I think this could still make sense as a capability if there is a practical application that is not well-served by alternatives? |
@dmontagu IMHO, I think this behaviour is befitting of pydantic's pre-v0.27 behaviour, on which Nevertheless, I do agree that this is a breaking change for people already using With that in mind, I propose the introduction of a flag in pydantic's |
A valid use case would be to make some data transformations that depend on the data type and the InitVars. Currently, you can do that on post_init, but you would have to manually check the type before making the operations/transformations using the InitVars. This is not optimal, since you would have to manually check something that pydantic will check anyway. If it were possible to access InitVars on _post_init_post_parse, this manual check would not be required. It seems fair to me that if there is a need for post_init to receive arguments in order to make some operation/transformation on the data before validation, then post_init_post_parse would also need to receive them for the same purpose. |
I personally don’t have a very strong opinion here as I generally prefer That said, I would find the change a lot more compelling if I saw a realistic-feeling example where I felt like there was a real benefit to having the initvars in the It wouldn’t take much to convince me (please share the use case if you have one!), but so far it just feels very abstract; I’m not sure it’s worth changing if no one is going to use it anyway. |
This makes sense to me in theory, but still feels very abstract. Do you have a real example? That might shut me up |
I definitely don't think the Regarding the feature: I'm in the same boat as @dmontagu, one decent concrete example and I'll accept this. |
Here is what an example of a use case would look like if I had to implement it on the current version of pydantic: import typing as T
from pathlib import Path
from dataclasses import InitVar
from pydantic import ValidationError, validator
from pydantic.dataclasses import dataclass
@dataclass
class Archive:
path: T.Union[Path, str]
base_path: InitVar[T.Optional[T.Union[Path, str]]] = None
@validator("path")
def validate_path(cls, v: T.Any) -> Path:
return Path(v)
def __post_init__(self, base_path):
if isinstance(self.path, Path) or isinstance(self.path, str):
base_path = Path(base_path)
self.path = base_path.joinpath(self.path)
else:
raise ValidationError(
f"unsupported type for path component: {type(self.path)}"
)
post_init = Archive("test.pdf", base_path="/usr/local")
print(post_init) I would have to manually validate the type of the property before doing the necessary operation... If the PR was accepted, the code would be the following: import typing as T
from pathlib import Path
from dataclasses import InitVar
from pydantic import validator
from pydantic.dataclasses import dataclass
@dataclass
class ArchivePostParse:
path: T.Union[Path, str]
base_path: InitVar[T.Optional[T.Union[Path, str]]] = None
@validator("path")
def validate_path(cls, v: T.Any) -> Path:
return Path(v)
def __post_init_post_parse__(self, base_path):
base_path = Path(base_path)
self.path = base_path.joinpath(self.path)
post_init_post_parse = ArchivePostParse("test.pdf", base_path="/usr/local")
print(post_init_post_parse) |
@Raphael-C-Almeida Your example has completely convinced me -- I now fully believe this should be supported in 1.0. I hadn't actually seen I'm going to add this example as a test to make it clear why this is useful. |
@dmontagu @samuelcolvin Considering @Raphael-C-Almeida 's example (a good one by the way), I think adding validation to InitVars should also be included in pydantic. This would render extra validations for InitVars in |
@HeavenVolkoff I think that would involve a relatively large refactor of
I don't think this is an unreasonable request, but this seems like a rather niche use case, could require a substantial amount of effort, and I'd guess the result would be substantially more fragile than the current implementation. I'd review a PR for it, but either way it should probably be separate from this one. |
@dmontagu I agree, this is a more complex request and you raised some valid points. I'll do a more in depth research on this matter to see if I can come up with a valid implementation to solve this in pydantic then open a new PR. |
great example, makes lots of sense, I've accepted the change. Regarding validating If you really wanted to validate |
Change Summary
Currently only
__post_init__
hasinitvars
support which is lacking on__post_init_post_parse__
, that's seams inconsistent considering the use case of both methods.My PR only add this functionality along with it's test case.
Checklist
changes/<pull request or issue id>-<github username>.rst
file added describing change(see changes/README.md for details)