Skip to content

Fix Validator protocol init to match runtime #1396

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Aug 11, 2025

Addresses #1382

The runtime init signature of a validator can be seen with inspect:

>>> import inspect, jsonschema
>>> inspect.signature(jsonschema.validators.Draft7Validator.__init__)
<Signature (self, schema: 'referencing.jsonschema.Schema', resolver=None, format_checker: '_format.FormatChecker | None' = None, *, registry: 'referencing.jsonschema.SchemaRegistry' = <Registry (20 resources)>, _resolver=None) -> None>

This aligns the protocol's declared signature with that behavior more exactly with the following changes:

  • registry is now keyword-only, not keyword-or-positional, and has a
    default.

  • resolver is added to the declared signature, so users who are using
    it won't see typing-time discrepancies with the runtime. It is marked
    as Any and commented inline as deprecated, since it's unclear what
    else we could do to indicate its status.
    This means that code passing a resolver will continue to type check
    (previously it would not).

  • resolver is the second keyword-or-positional and format_checker is
    the third, meaning that a positional-only caller who passes, for
    example: Draft202012Validator(foo, None, bar) will have foo
    slotted as the schema and bar as the format_checker
    This would primarily impact callers with positional-args calling
    conventions, but is more reflective of what they'll see at runtime.

In order to remove resolver from the protocol signature, but match the runtime signatures well, some kind of placeholder is needed to indicate format_checker as positional-or-keyword.
Or else, a large number of overloads for __init__ could be declared to try to simulate its removal.

I considered such options but decided that they add undue complexity for what is supposed to be a hint for correct usage.

Assuming this or something like it merges, I'll backport to typeshed.


📚 Documentation preview 📚: https://python-jsonschema--1396.org.readthedocs.build/en/1396/

@Julian
Copy link
Member

Julian commented Aug 12, 2025

Thanks! Merging main will fix the license check, I still haven't gotten a chance to write a new tool unfortunately :/

The change looks correct obviously, but I'd love to have some actual tests... When I tried to fix this a few weeks ago I added a jsonschema/tests/typing_tests.py with:

"""
Static typing tests for jsonschema.
"""

from typing import assert_type

from jsonschema import Draft202012Validator
from jsonschema.protocols import Validator

assert_type(Draft202012Validator, type[Validator])

to at least address this specific test.

I forget what issue I ran into which made me unable to just add that immediately and fix this. Does it pass after your change here or do we run into some other broken annotation? If it passes (or if you otherwise have a recommendation for what to add to do this) could you add that as well?

The runtime init signature of a validator can be seen with `inspect`:

```pycon
>>> import inspect, jsonschema
>>> inspect.signature(jsonschema.validators.Draft7Validator.__init__)
<Signature (self, schema: 'referencing.jsonschema.Schema', resolver=None, format_checker: '_format.FormatChecker | None' = None, *, registry: 'referencing.jsonschema.SchemaRegistry' = <Registry (20 resources)>, _resolver=None) -> None>
```

This aligns the protocol's declared signature with that behavior more
exactly with the following changes:

`registry` is now keyword-only, not keyword-or-positional, and has a
default.

`resolver` is added to the declared signature, so users who are using
it won't see typing-time discrepancies with the runtime. It is marked
as `Any` and commented inline as deprecated, since it's unclear what
else we could do to indicate its status.
This means that code passing a resolver will continue to type check
(previously it would not).

`resolver` is the second keyword-or-positional and `format_checker` is
the third, meaning that a positional-only caller who passes, for
example: `Draft202012Validator(foo, None, bar)` will have `foo`
slotted as the schema and `bar` as the `format_checker`
This would primarily impact callers with positional-args calling
conventions, but is more reflective of what they'll see at runtime.

In order to remove `resolver` from the protocol signature, but match
the runtime signatures well, some kind of placeholder is needed to
indicate `format_checker` as positional-or-keyword. Or else, a large
number of overloads for `__init__` could be declared to try to
simulate its removal.
@sirosen sirosen force-pushed the improve-protocol-init-signature branch from 6b37b3f to a916d8f Compare August 12, 2025 20:08
@sirosen
Copy link
Member Author

sirosen commented Aug 12, 2025

I'd love to have some actual tests

Yeah, agreed. Given that there wasn't existing art to follow I didn't know about adding something new. But I'm happy to experiment a little and try to add something lightweight.

I forget what issue I ran into which made me unable to just add that immediately and fix this. Does it pass after your change here or do we run into some other broken annotation? If it passes (or if you otherwise have a recommendation for what to add to do this) could you add that as well?

I'll need to try it out to be sure, but I think assert_type is stricter than we want here.
I only know this because I've been corrected by typing experts on it, but apparently assert_type requires an exact match, not a subtype.

e.g.,

x: int = 1
assert_type(x, int | str)  # fails, because `x` is `int`, not `int | str`

But subtypes are what's checked by looking at what's assignable:

x: type[Validator]
x = Draft202012Validator  # only passes if `Draft202012Validator` is a subtype of `Validator`

I'll have to try some stuff out and I'll report back when I can make a viable test.

@Julian
Copy link
Member

Julian commented Aug 12, 2025

OK, cool thanks! Will wait to hear back. I guess given we have a runtime_checkable Protocol, we could just isinstance it too?? But will wait to hear if you come up with anything.

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.

2 participants