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

Improve annotation of jsonschema.validators.create #8608

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

sirosen
Copy link
Contributor

@sirosen sirosen commented Aug 24, 2022

I'm trying to resume annotating this library this summer/fall, so I'm putting in a small change to start.

@sirosen sirosen force-pushed the jsonschema-add-create-signature branch 2 times, most recently from fbd11ba to 4138114 Compare August 24, 2022 23:31
@github-actions

This comment has been minimized.

@sirosen sirosen force-pushed the jsonschema-add-create-signature branch from 4138114 to 989bd8b Compare August 24, 2022 23:35
@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

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! A few remarks below.

stubs/jsonschema/jsonschema/validators.pyi Outdated Show resolved Hide resolved
stubs/jsonschema/jsonschema/validators.pyi Outdated Show resolved Hide resolved
stubs/jsonschema/jsonschema/validators.pyi Outdated Show resolved Hide resolved
stubs/jsonschema/jsonschema/validators.pyi Outdated Show resolved Hide resolved
@sirosen sirosen force-pushed the jsonschema-add-create-signature branch from decdaca to 817eff4 Compare August 25, 2022 13:17
@sirosen
Copy link
Contributor Author

sirosen commented Aug 25, 2022

I did a quick amend because I realized I missed one of the _JsonObject -> _Schema changes. Thanks for the review!

I still need to work out how to handle these type aliases and annotations in the jsonschema source. It's likely possible that in jsonschema we'll end up with _JsonValue, but perhaps not _Schema (I may simply suggest Mapping[str, Any] at those usage sites). It will depend a bit on how much the maintainer has an opinion about this.

EDIT: clarified my meaning a bit.

@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. 🤖🎉

@srittau srittau merged commit 2c534dc into python:master Aug 25, 2022
@sirosen sirosen deleted the jsonschema-add-create-signature branch August 25, 2022 15:36
sirosen added a commit to sirosen/jsonschema that referenced this pull request Dec 7, 2022
python/typeshed#8608 introduced annotations for `create` which are not
fully reflected here.

In order to reflect that state into `jsonschema`, a new module,
`jsonschema._typing` is introduced. The purpose of `_typing` is to be
a singular place for the library to define type aliases and any
typing-related utilities which may be needed. This will let us use
aliases like `_typing.JsonValue` in many locations where any valid
JSON datatype is accepted.
The definitions can be refined over time as necessary.

Initial definitions in `_typing` are:
- Schema (any JSON object)
- JsonObject (any JSON object)
- JsonValue (any JSON value, including objects or sequences)

`Schema` is just another name for `JsonObject`. Perhaps it is not
needed, but the name may help clarify things to a reader. It is not
obvious at present whether or not it is a good or bad idea to notate
it as such, but a similar Schema alias is defined in typeshed and
seems to be working there to annotate things accurately.

These types are using `Mapping` and `Sequence` rather than `dict` or
`list`. The rationale is that `jsonschema`'s logic does not dictate
that the objects used *must* be defined in stdlib types or subtypes
thereof. For example, a `collections.UserDict` could be used and
should be accepted by the library (`UserDict` wraps but does not
inherit from `dict`.)
sirosen added a commit to sirosen/jsonschema that referenced this pull request Dec 7, 2022
python/typeshed#8608 introduced annotations for `create` which are not
fully reflected here.

In order to reflect that state into `jsonschema`, a new module,
`jsonschema._typing` is introduced. The purpose of `_typing` is to be
a singular place for the library to define type aliases and any
typing-related utilities which may be needed. This will let us use
aliases like `_typing.JsonValue` in many locations where any valid
JSON datatype is accepted.
The definitions can be refined over time as necessary.

Initial definitions in `_typing` are:
- Schema (any JSON object)
- JsonObject (any JSON object)
- JsonValue (any JSON value, including objects or sequences)

`Schema` is just another name for `JsonObject`. Perhaps it is not
needed, but the name may help clarify things to a reader. It is not
obvious at present whether or not it is a good or bad idea to notate
it as such, but a similar Schema alias is defined in typeshed and
seems to be working there to annotate things accurately.

These types are using `Mapping` and `Sequence` rather than `dict` or
`list`. The rationale is that `jsonschema`'s logic does not dictate
that the objects used *must* be defined in stdlib types or subtypes
thereof. For example, a `collections.UserDict` could be used and
should be accepted by the library (`UserDict` wraps but does not
inherit from `dict`.)
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

2 participants