-
-
Notifications
You must be signed in to change notification settings - Fork 656
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
✨ Support common pydantic types #723
base: master
Are you sure you want to change the base?
Conversation
📝 Docs preview for commit 7ea8313 at: https://d7891fe3.typertiangolo.pages.dev |
📝 Docs preview for commit a36f24d at: https://9028cd14.typertiangolo.pages.dev |
0589187
to
23988ad
Compare
📝 Docs preview for commit 23988ad at: https://4451bb64.typertiangolo.pages.dev |
23988ad
to
08d1fac
Compare
📝 Docs preview for commit 08d1fac at: https://3976aef9.typertiangolo.pages.dev |
08d1fac
to
f6c2b03
Compare
📝 Docs preview for commit f6c2b03 at: https://1d1a6c3f.typertiangolo.pages.dev |
debccf1
to
2a48587
Compare
Hi! Note that Python 3.6 support was recently dropped in 0.11.0. |
Yep I noticed that's why I rebased my PR yesterday 😁 |
There's an issue with the documentation building on the CI, unrelated to this PR. We're looking into it! [UPDATE] docs building CI error is fixed. |
📝 Docs preview for commit 19319c6 at: https://99282d37.typertiangolo.pages.dev |
Looks great, I'd love to see this get merged! Could we somehow also support custom types defined by users like pydantic? |
Me too 😁
TBH I think this PR covers most of cases we need for now and I prefer to keep it minimalistic to get a chance for it to be merged soon. |
📝 Docs preview for commit fde3d16 at: https://6fef5bb9.typertiangolo.pages.dev |
📝 Docs preview for commit 2166319 at: https://ddf226eb.typertiangolo.pages.dev |
📝 Docs preview for commit 0ac6ec0 at: https://d8df500d.typertiangolo.pages.dev |
📝 Docs preview for commit 707839e at: https://ad610314.typertiangolo.pages.dev |
@tiangolo Is this PR being considered? |
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.
@lachaib : I've internally solicited feedback from @tiangolo to decide on the direction of this work. The PR is in a good state to review and to assess what the added functionality would offer. But he'll need to decide whether we want to add this optional Pydantic dependency to Typer. I basically see three options:
- Add Pydantic as default, but optional dependency together with
rich
andshellingham
(as in this PR) - Add Pydantic as non-default, optional dependency through an additional install flag (this PR can be edited to do that)
- Don't add Pydantic as a dependency and not have this built-in support
If the third option is chosen, an alternative such as @pypae's recent WIP on pydantic-typer
could be considered.
Given this feature is listed in the roadmap #678 , I pray this won't end up in option 3. |
📝 Docs preview for commit 795e8e4 at: https://7eb4439c.typertiangolo.pages.dev Modified Pages |
📝 Docs preview for commit 9fa6717 at: https://b43ba1fa.typertiangolo.pages.dev Modified Pages |
Slightly tricky merge conflict - I'll look into this and get this branch back up-to-date and green 🤞 Update: the test failure came from reverting an edit in this PR that changed the imports for |
9bd66be
to
efa434b
Compare
efa434b
to
0a793f6
Compare
@pypae just so you know, I've tried to play a little bit yesterday night on getting this PR extended to support Custom Types, but the stack is stripping most annotations all the way down to the moment we're calling |
This PR is "functional" as in "it does what was expected"
However, don't hesitate to request more tests, docs, or improvements on the code if needed to fit with the current standards
Fixes #181