Codecov Report

Merging #954 (0d20be2) into main (d715573) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #954   +/-   ##
=======================================
  Coverage   96.88%   96.88%           
=======================================
  Files          20       20           
  Lines        3276     3276           
  Branches      449      449           
=======================================
  Hits         3174     3174           
  Misses         79       79           
  Partials       23       23           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d715573...0d20be2. Read the comment docs.

Thanks.

Apologies for being late on this, but I saw this as the latest commit on main. Is this correct? jsonschema is in typeshed and does not include annotations for most of its own types.

I feel responsible as the person who introduced mypy and initial annotations to this project. I very intentionally -- out of respect for this project's current stance on typing -- did not take this measure or even start work towards it.

I believe, although I'm not certain, that this should be reverted.

Sorry! I misread the name of the person who approved this as yours, so that's me being sloppy. If this is incorrect I'll indeed revert!

I think reverting is for the best, if there haven't been more changes to support it. It's misleading at best and might take precedence over the typeshed stubs at worst.

I hope to gradually change your mind about typing and one day introduce this. However, part of that strategy of convincing you that the typing stuff is valuable is to not cause disruption here.

Good strategy :)

Sounds good thanks for the advice! Definitely open to gradually getting there.

Done. Thanks for catching!

I am even more confused, I was not expecting why having the lib not typed is desired. Typeshed is more of historical hack that does not scale and it is always outdated.

It's just not something that's been high priority. There are other more important issues to work on in the queue. Small self-contained patches are welcome.

I could try to make few such patches to help with this. Adding type is tricky and hard to prioritise, a gradual approach worked for the projects I work on proved to work decently. There are even typing challenges around JSON arguments and return types which are not yet addressed.

There were some amazing and reactively unexpected benefits that I found since I started to use types everywhere, forced a much better design when writing code.

I was not expecting why having the lib not typed is desired. Typeshed is more of historical hack that does not scale and it is always outdated.

In an issue discussion I recall Julian being wary of taking on extra burdens in jsonschema in order to support type annotations. Small and incremental, as you have suggested, is definitely the way to go.

I agree that typeshed is a temporary workaround, not a long-term solution. I only mention it because that is the current location for type annotations for jsonschema, so we need to be aware of that.

[using] types everywhere, forced a much better design when writing code.

I've found this as well, but I've also found that idiomatic or even very good designs can conflict with the limitations of type checkers.

Sounds like we're all mostly in agreement :) if/when it turns out type checking reveals either a bug or a better design, that certainly will help me come around -- until then I view it basically as a documentation improvement, and yeah there are lots of docs improvements I want to make, this being just one of em. But yeah I appreciate both of your help!

@Julian Maybe we can find a way to address this?

Apparently 4.6.0 already broke integration because some types were changed and the types-jsonschema is heavily outdated:

    ERROR: Could not find a version that satisfies the requirement types-jsonschema>=4.6.0 (from versions: 3.2.0, 3.2.1, 4.2.0, 4.3.0, 4.3.1, 4.3.2, 4.4.0, 4.4.1, 4.4.2, 4.4.3, 4.4.4, 4.4.5, 4.4.6, 4.4.7, 4.4.8, 4.4.9)

Basically I cannot run mypy on my code with jsonschema 4.6.0 because there are no types for it and the change to add them was reverted.

Personally I do not think that maintaining types separately is sustainable at all. Maybe it was ok 2-3 years ago when types were not so well spread and for libraries that were not under active maintenance. Still for any software that is actively maintained, like this library it worth not offloading the types to someone else.

The errors that I seen were related to a type mismatch for jsonschema.ValidationError.absolute_path which looked like:

 has incompatible type "deque[Union[str, int]]"; expected "deque[str]"

When I went to the source code, I seen a method without any types on it. To guess which types were involved it would require a lot of digging and grepping around the source code.

the change to add them was reverted.

As I understand it (as it was explained to me really), the change here was just purely incorrect, it marks this library as shipping types when it does not yet. Again, such PRs are welcome in small chunks.

Super. I will include the stub files from typeshed and that should make the PR valid. We can wait for others to check it before merging as we don't want to break it.

I have been working with the typeshed maintainers to improve the stubs, and I have been working on finding good candidates to add to mypy_primer.

The stubs are not badly out of date, and I'm not sure what gives you the impression that they are. I just got a PR merged to update them in the last week or two, and I followed up here by starting to get the format checker code annotated.

There is an incremental path I'm following here to get the stubs gradually more fleshed out, leveraging the expertise of the typeshed team and the infrastructure they have with mypy_primer and other CI. As we expand the stubs, I'll be working here to backport the annotations and resolve any inconsistencies and failures.

Please do not add a copy of the stubs here. It will only get in the way of the active effort to add annotations.

@sirosen I am quite curious how would you do a progressive/stepped switch without this approach because adding any type info to the library itself would not be testable by users once they are already forced to use typeshed and the lack of py.typed marked prevents them to use the new types.

I see this as a chicken and the egg problem and IMHO doing a copy from typeshed allows us to migrate these into code gradually, after this move, like one file at a time.

The goal is to develop fully fleshed out stubs in typeshed, backporting those annotations to jsonschema as we go. The approach was laid out here: python/typeshed#7950 (comment)

There were a small number of errors in those annotations. I regret that, but they were found and fixed quickly because the stubs are actively used. These PRs fixed issues:
python/typeshed#7980
python/typeshed#7981
python/typeshed#7990

The typeshed owners also made this adjustment:
python/typeshed#8024

I also added check-jsonschema to mypy_primer:
hauntsaninja/mypy_primer#35

If there are other libraries which should be added to mypy_primer, that would be most welcome. Note that mypy_primer does not support the use of mypy plugins at present.

The TypeAlias change is a good demonstration that there is expertise in typeshed which is worth leveraging. typeshed also has some capabilities which are not broadly available in the _typeshed imports, which allows them to run ahead of what is provided in typing_extensions and mypy with things like their custom Self and Comparable types.

@Julian Would you mind removing Mark library as typed (PEP-561) by @ssbarnea in https://github.com/python-jsonschema/jsonschema/pull/954 from the release description? It's a bit confusing since it has been reverted 🙃

Done!

That was fast 🚀 ! Thank you 😃