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

Add jsonschema libs #35

Merged
merged 5 commits into from
Jun 4, 2022
Merged

Conversation

sirosen
Copy link
Contributor

@sirosen sirosen commented May 30, 2022

I'm adding more info to types-jsonschema stubs, and there were two bugs which it would have been nice to catch earlier. Add two users of types-jsonschema to the project list.

@DMRobertson, I don't believe that adding your project to mypy_primer carries any obligation for you or your team, but wanted to give you a heads-up about this.

We can also cut synapse and just add check-jsonschema if adding it is an issue for someone.

sirosen added 2 commits May 30, 2022 16:51
Synapse devs noted an issue with types-jsonschema as types are being
introduced to the barebones stubs in typeshed. Checking against their
usage also seems to cover a bunch of other libraries in typeshed.
@DMRobertson
Copy link

@DMRobertson, I don't believe that adding your project to mypy_primer carries any obligation for you or your team, but wanted to give you a heads-up about this.

We can also cut synapse and just add check-jsonschema if adding it is an issue for someone.

Thanks for the heads-up! No objections from us, but I would like to sound a few notes of caution:

Co-authored-by: David Robertson <david.m.robertson1@gmail.com>
@sirosen
Copy link
Contributor Author

sirosen commented May 30, 2022

I think that it's okay that not everything inside the library is annotated, so long as it passes mypy without errors. A bunch of Anys is okay; failing mypy probably isn't (though I don't know for sure).

More importantly, I saw that you're using a bunch of different stubs from typeshed. IMO, that makes the project a sort of attractive option to include, since it covers many different parts of typeshed, even if it only uses each one a little.

This seems to me like the biggest likely issue. I'll wait to hear from maintainers about this, but I probably wouldn't have suggested the integration if I'd known that there's an internal plugin. The internal plugin in particular could make mypy_primer overly sensitive to changes in that project.

Everything else seems pretty much okay to me. I see a commented out block where one project was listed as pulling the types-* packages out of a setup.py, but everything else seems vulnerable to the issue of getting out of date on needed stubs.

@JelleZijlstra
Copy link
Collaborator

See #8 for discussion of plugins. But I think it's fine if mypy-primer projects get some mypy errors: what matters is that they give us useful information if something changes.

This repo uses mypy plugins including a custom, internal plugin. As
such, it isn't suitable for testing under mypy_primer until/unless
plugin handling is added.
@sirosen
Copy link
Contributor Author

sirosen commented May 31, 2022

I removed synapse since it sounds like it can't be added (yet), so this PR is down to just check-jsonschema for now.

I've started looking for other projects using types-jsonschema, but I'll save any additions for a future change.

@hauntsaninja hauntsaninja merged commit 96376fb into hauntsaninja:master Jun 4, 2022
@hauntsaninja
Copy link
Owner

Thanks! I will one day get around to adding support for plugins :-)

@sirosen sirosen deleted the add-jsonschema-libs branch June 5, 2022 21:19
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.

4 participants