Skip to content
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

Fix exception types for jsonschema._format #7990

Merged
merged 4 commits into from
May 30, 2022

Conversation

sirosen
Copy link
Contributor

@sirosen sirosen commented May 30, 2022

The annotated type for the raises argument on format checkers was

Exception | tuple[Exception, ...]

when it should read

type[Exception] | tuple[type[Exception], ...]

This is a mistake I introduced in #7950 . So far, there have been two or three issues found with those changes, depending on how you count. I will try to make smaller changes to jsonschema stubs in the future to avoid creating this situation again.

The annotated type for the `raises` argument on format checkers was

    Exception | tuple[Exception, ...]

when it should read

    type[Exception] | tuple[type[Exception], ...]
@@ -2,14 +2,15 @@ from collections.abc import Callable, Iterable
from typing import Any, TypeVar

_F = TypeVar("_F", bound=Callable[..., Any])
_RaisesType = type[Exception] | tuple[type[Exception], ...]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've hit a known mypy bug (python/mypy#11098, python/mypy#12392) in its handling of PEP-604 type aliases. The workaround is to use the old syntax (Union) here instead. The rest of our CI won't complain about it in this instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, easy enough -- I'll have that done in just a second.

Also, thanks to you and everyone on typeshed for being super responsive! It makes it very easy to contribute. ❤️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks to you and everyone on typeshed for being super responsive! It makes it very easy to contribute. ❤️

I'm very glad to hear that! Also, don't beat yourself up too much about the regressions — it's nearly inevitable if you're adding annotations to stubs that currently don't have many type hints at all. One way we could work to reduce these in the future, however, would be to add more open-source projects using types-jsonschema to mypy_primer (https://github.com/hauntsaninja/mypy_primer)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give that a shot. check-jsonschema is where I'm doing my own work.
And there was a PR from the maintainers of https://github.com/matrix-org/synapse , so perhaps I can integrate that as well. I'll have to see how they have mypy setup, of course.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

stubs/jsonschema/jsonschema/_format.pyi Outdated Show resolved Hide resolved
@srittau
Copy link
Collaborator

srittau commented May 30, 2022

Sorry, my bad, TypeAlias needs to be imported from typing_extensions.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit 17e7cc7 into python:master May 30, 2022
@sirosen sirosen deleted the jsonschema-format-fix-raises branch May 30, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants