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
Add support for Literal annotation #582
Conversation
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.
I'm keen on the idea, just a few things to fix.
Codecov Report
@@ Coverage Diff @@
## master #582 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 2582 2613 +31
Branches 510 516 +6
=====================================
+ Hits 2582 2613 +31 |
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.
also need to:
- add this to docs
- update travis to run with and without
typing_extensions
, same as it currently does forujson
etc.
Otherwise this is looking great.
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.
Looking good, just a few small things then please add to history and docs.
I'm confused about why the coverage appears to have dropped, maybe that is just codecov getting confused.
This pull request introduces 2 alerts when merging c5ac4e7 into 461b852 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert when merging 9a3cd2d into 461b852 - view on LGTM.com new alerts:
|
@samuelcolvin thanks for merging this, I'll add docs soon |
great thanks, somehow forgot about that. |
Great job all! |
Change Summary
Properly handle attributes annotated with
typing_extensions.Literal
If you like this, let me know and I'll add documentation and finish the other bullets in the checklist. I'd also be happy to try implementing a workaround to eliminate the
typing_extensions
dependency (given some guidance about the desired outcome). I could also look into how to better integrate with the schema.If you don't like this (e.g., because you think my implementation is awkward, or because you think this feature is just premature while
Literal
remains intyping_extensions
rather thantyping
), no worries -- I'll just close the pull request and maybe come back to it at a later date. But the implementation seemed simple enough that I felt like it was worth sharing.Related issue number
#561
Checklist
HISTORY.rst
has been updated#<number>
@<whomever>