-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Updated Stubs for jsonschema #6486
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
Conversation
srittau
left a comment
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 for updating the stubs! A few remarks below.
srittau
left a comment
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 for the fixes. Two minor issues below then this is good to go!
This PR updates stubs to recognize new attributes added since the original stubbing in python#5784.
Co-authored-by: Sebastian Rittau <[email protected]>
a95597d to
4ad187c
Compare
|
Nit: Avoid |
4ad187c to
50b96cc
Compare
As recommended by @Akuli
Noted, I'll do that in the future. |
7236a0c to
0c7803c
Compare
|
Derp. I've updated my git local configuration to avoid needing to force push in this repo. Sorry for the error. |
| def is_ipv4(instance) -> bool: ... | ||
| def is_ipv6(instance) -> bool: ... | ||
|
|
||
| # is_host_name is only defined if fqdn is installed. |
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.
Nit: It would be better to put these comments to the allowlist. You can add comments to end of line, e.g.
jsonschema._format.is_host_name # defined only if fqdn is installed
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 actually like them in the source file as well as the allowlist is basically invisible when developing with the stubs, while the comment in the source are visible when jumping into the stub file.
This PR updates stubs to recognize new attributes added since the
original stubbing in #5784.