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

Are PRs that add typing to the code appreciated? #1017

Closed
DanielNoord opened this issue Nov 25, 2022 · 5 comments
Closed

Are PRs that add typing to the code appreciated? #1017

DanielNoord opened this issue Nov 25, 2022 · 5 comments

Comments

@DanielNoord
Copy link
Contributor

Basically the issue title: are such PRs welcomed? The stubs in typeshed are outdated and instead of updating them there I thought I could also invest some time in the project itself so they are more readily available for everybody.

@Julian
Copy link
Member

Julian commented Nov 25, 2022

If done incrementally (in small chunks) and in a way that doesn't make the code significantly worse (locally or in aggregate) yes!

@sirosen has some plans there too which may be worth coordinating.

And appreciate both the offer and you asking!

@Viicos
Copy link

Viicos commented Mar 6, 2023

If I can get any free time, I'll be happy to work on adding types on this library. Just wanted to know if there was any rules we should follow? (e.g. are we going for mypy --strict? Do we need explicit return types?)

@sirosen
Copy link
Member

sirosen commented Mar 6, 2023

I'll chime in to say that I have not had the kind of free time in the past ~6 months that I expected to have to work on type annotations for jsonschema. I put forth a plan on typeshed for this which might not be wise/relevant because it was predicated on me having a lot of time to carefully sync the two sources of truth (see also: #1027).

Here's my current take (about which I would also like @Julian's feedback):

  • If you see something where there is a clear and correct annotation to add, please add it! I will be happy to help review/discuss.
  • Smaller is better. We can add good/correct annotations here and then I can port them to typeshed as much as I'm able. The smaller they are, the easier that porting process will be. It also avoids hang-ups about ambiguous typing choices (e.g. MutableMapping vs dict) blocking larger chunks of work.
  • Let's not publish a py.typed file until we have figured out criteria for doing so. I don't want to cause regressions for people relying on typeshed, but holding this back too much may do more harm than good. It needs thought.

typeshed gets run through mypy_primer, which is really useful. I'd really like to find a way to run mypy_primer against our changes before publishing a py.typed file for jsonschema.

@Viicos
Copy link

Viicos commented Mar 6, 2023

Yes sure and I think #1019 (comment) has to be taken into account, hence I think I'll go for the most straightforward ones first.

typeshed gets run through mypy_primer, which is really useful. I'd really like to find a way to run mypy_primer against our changes before publishing a py.typed file for jsonschema.

This is possible locally, I did it once but it is pretty heavy to run iirc

@Julian
Copy link
Member

Julian commented Mar 9, 2023

Hi there! Definitely agree on my side on the straightforward ones being easiest to get in, so those are definitely welcome!

I have to teach myself what mypy_primer is -- honestly my experience getting referencing written with types led me to basically ditch mypy and switch to using pyright which had some better defaults IMHO -- but indeed I think there's likely plenty of low hanging fruit we can get to here!

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

No branches or pull requests

4 participants