-
Notifications
You must be signed in to change notification settings - Fork 96
Added pydantic integration for Version #343
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 pydantic integration for Version #343
Conversation
- Added two new dunder methods in Version which comply with the custom field types described [here](https://pydantic-docs.helpmanual.io/usage/types/#classes-with-__get_validators__). - Updated usage documentation with example for using with Pydantic. - Added test case for pydantic integration. - Added pydantic dependency in `tox.ini` for tests cases. - Verified all tests passed for all python versions with `tox`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @calebstewart for your efforts and sorry for the delay! Much appreciated.
I never heard about Pydantic, but it seems to be an interesting project. Not entirely sure if this is something for this project, but I think we should give it a try. 🙂
I've added some small, minor comments. If you could have a look we could merge them.
After looking at this PR again and thinking about it, I'm sorry but I'm not entirely convinced we should amend the class with these methods. Although the overall change is simple and doesn't introduce any runtime dependencies (which is good!), I have the feeling it doesn't add any value to non-Pydantic users. How many users really use semver and pydantic in this way? What should we do if another user have the same idea, but with a different library? Or what happens if Pydantic introduces some incompatible changes? Where do we draw the line? Wouldn't that lead to a "bloated" class? I know, it's a bit of a stretch. However, my philosophy in this regard is that I try to keep any runtime dependencies away from semver. It shouldn't depend on other libraries as the overall scope is quite focused. So in other words, it should stand on its own legs. Maybe we should follow this approach with "virtual" dependencies too. Have you tried to solve your issue by inheritance? class PydanticVersion(Version):
@classmethod
def __get_validators__(cls):
"""Return a list of validator methods for pydantic models."""
yield cls.pars
@classmethod
def __modify_schema__(cls, field_schema):
"""Inject/mutate the pydantic field schema in-place."""
field_schema.update(examples=["1.0.2", "2.15.3-alpha", "21.3.15-beta+12345"]) Then you could add this to your model: class MyModel(pydantic.BaseModel):
version: PydanticVersion I haven't tried it, but it looks like a small cost. However, I don't want to completely reject it. I'd like to suggest to include it into the documentation. It could serve as an example about how we could combine semver with other projects. Let me know if you have a compelling argument that I've overlooked. I'd really like to know! I leave this PR open for the time being. |
I apologize for never getting back to you on those requests. The holidays were kind of crazy, and this fell off my plate. I totally understand the apprehension here. I wasn't completely sure if it fit either, to be honest. Not a huge deal. I think your example looks right as well. That is what I ended up doing in the end. If you'd like to add something like that as a note/example in the documentation, I think that would be really cool and possibly helpful! I have no problem with this PR being closed, though. I totally understand your position. 👍 |
All good, don't worry. I was also busy with other stuff. 😉 Thank you Caleb for your nice words! I'm really sorry, hope it wasn't too much effort on your side. I'd proceed as follows: I'll integrate a new section into the documentation describing the above finding. @calebstewart Would you like to review it once it's finished? |
Related to issue python-semver#343
Closing this PR in favor of #353 |
Related to issue python-semver#343 Co-authored-by: <calebstewart@users.noreply.github.com>
Related to issue python-semver#343 Co-authored-by: Caleb Stewart <calebstewart@users.noreply.github.com>
This PR adds native pydantic integration to
python-semver
. Pydantic makes defining data schemas/models super easy and intuitive, but doesn't have a native field type for semantic version strings. Custom field types are created by simply including a class method__get_validators__
on the class which yields methods to validate input syntax and return the resulting type. In this case, theparse
method functions naturally as the field validator. Pydantic also allows the__modify_schema__
method to inject extra details about the field into the model schema.In this case, I simply added the
__modify_schema__
and__get_validators__
methods to theVersion
class. These two class-methods are one line each. The former injects semantic versioning examples into the generated schema while the latter yields theVersion.parse
method as a validator. The result is that a Pydantic user can do the following:It's worth noting that a
pydantic
user could manually specifyVersion.parse
as a validator for the field in their model, but this is less expressive, and the overhead for makingsemver.Version
compatible withpydantic
is relatively low (two one-line class methods).That being said, I totally understand if you think this is out of scope here, but it seemed like a simple quality of life improvement, and thought I'd throw it out there. I went through the contributing guidelines, and I think I've hit all the marks. Thanks for reviewing/considering! 😃
Changes:
field types described here.
pydantic
.pydantic
integration (mainly copied parsing tests, and modified to testpydantic
integration).pydantic
dependency intox.ini
for tests cases.tox
.