Skip to content

Fix RefResolver store parameter type in jsonschema stubs#15542

Open
sedat4ras wants to merge 1 commit intopython:mainfrom
sedat4ras:fix/jsonschema-refresolver-store-type
Open

Fix RefResolver store parameter type in jsonschema stubs#15542
sedat4ras wants to merge 1 commit intopython:mainfrom
sedat4ras:fix/jsonschema-refresolver-store-type

Conversation

@sedat4ras
Copy link

Fixes #8662

Changes

In stubs/jsonschema/jsonschema/validators.pyi, the store parameter of RefResolver.__init__ was typed as SupportsKeysAndGetItem[str, str] | Iterable[tuple[str, str]], but the actual runtime implementation accepts a mapping/iterable of Mapping[str, Any] (i.e., full JSON schema objects), not plain str values.

The jsonschema source code uses URIDict (which inherits from MutableMapping[str, Any]) for the store, and the store values are schema documents (JSON objects), not strings.

Before:

store: SupportsKeysAndGetItem[str, str] | Iterable[tuple[str, str]] = ...,

After:

store: SupportsKeysAndGetItem[str, Mapping[str, Any]] | Iterable[tuple[str, Mapping[str, Any]]] = ...,

This matches the actual usage pattern where users pass schema documents as store values:

RefResolver("http://example.com/", schema, store={"http://example.com/other": other_schema})

@github-actions
Copy link
Contributor

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

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Thanks, one request below.

Also, I noticed that URIDict in _utils.pyi also derives from MutableMapping[str, str]. This should probably also be changed to MutableMapping[str, MutableMapping[str, Any]] (and of course some method types need to be changed as well.) You could include these changes in this PR if you want to, but that's optional.

base_uri: str,
referrer: dict[str, Incomplete],
store: SupportsKeysAndGetItem[str, str] | Iterable[tuple[str, str]] = ...,
store: SupportsKeysAndGetItem[str, Mapping[str, Any]] | Iterable[tuple[str, Mapping[str, Any]]] = ...,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed something else: values() is called on store if it's a Mapping:

https://github.com/python-jsonschema/jsonschema/blob/a7277432b0f7bcd0551f6e589d30457017125df4/jsonschema/validators.py#L948-L952

While we usually try to avoid Mapping in new annotations, considerung that RefResolver is deprecated and will be removed at some point, just using it here is probably the most pragmatic solution.

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.

[jsonschema] RefResolver(store) argument misannotated

2 participants