Repository navigation
Support for typing? #2625
Description
Activity
Type hints would be great. I am using vscode and linting helps a lot.
I am willing to contribute.
Reacted by Cees Timmerman and UP929312I am as well. I think this would be a great addition to the project.
It sounds like a good idea, at least at the top 'interface' level to help ensure a well defined interface.
Typeshed would be a simple way to get it up and running without much interference with the current repo, but in the long run I think that the appropriate place for the type annotations would be in this repo, so that they can be kept in sync and (potentially at least) help keep bugs out of the code base in the testing phase.
What problems would the project face with Python 2/3 compatibility?
I've been working with mypy in Zulip, and at least until recently they use 2 and 3, and mypy seems to work for that.
@wiredfool I'll explore expanding typeshed then, if that's a go-ahead - typeshed is clear on seeking approval from project maintainers first.
That said, if direct annotation PRs would be accepted, that'd be something I'd look at working on instead.
I'm not up on the technical bits for doing type annotations. But, as I understand, there are three ways to do it:
- In the function signatures. Requires 3.5ish+. Not an option here due to our continuing support for older pythons.
- In function docstrings.
- In Typeshed.
The advantage of docstrings is that they're next to the code, and once it's all in, it can be maintained and not drift out of sync with the code. The disadvantage is that they're going to require PRs into this repo.
Can you do a function or two as an example against Image.py where we've got autodoc extracting the documentation? And then possibly see if we can test against that in the test suite?
I wouldn't opt for writing type annotations in docstrings since it's so easy to edit code and forget to update them if the types should change, even marginally. I do, however, like the methods mentioned here. Adding single (or multiple) line comments describing a function or method's type just before a docstring is checked by Mypy (and highlighted by Pycharm).
Annotating in docstrings is maybe coming to mypy through a plugin, IIRC. However, yes, I was just going to take a look at this using comments.
From my POV docstrings and a comment prior to the docstring are roughly equivalent, They're both not 'in code' to the point of breaking on 2.7, but they're in the same source file.
We'd need to make the typing module a conditional import, as it doesn't need to be required.
I'm working on this now.
Provisional work is now in my fork here https://github.com/neiljp/Pillow/tree/test_annotate
Should I make a WIP PR? That's a lot of functions with annotations, but not all quite fully there, and mypy doesn't 'pass' the code fully (and that's just with that file). There are some code changes which I could make to help the code pass and improvements/hints from mypy, but I wasn't sure whether to just make annotations, or perhaps also add related potential changes to the code but in separate commits in the same PR?
Anyhow, interested to hear what the feeling is so far.
Go ahead and make a PR for that. It'll give us a good place to discuss it, I can ask questions and hopefully arrive at something that works/passes.
Any reason why this is open and abandoned ?
9 remaining items
To be clear, the type hints are not yet complete (e.g.
PIL.Imagehas only a few type hints), but a large part of Pillow now has complete type hints.Thanks for everyone who made this possible!
Even if types are partially incomplete, it's a great base on which smaller contributions can be added as needed :)
Reacted by Hugo van Kemenade and Neil GirdharThank you so much everyone who contributed to this.
To be clear, the type hints are not yet complete (e.g.
PIL.Imagehas only a few type hints), but a large part of Pillow now has complete type hints.Thanks, yes, we'll still need to add some, and likely adjust some newly-added ones, but we've added the PEP 561
py.typedmetadata so I think this tracking issue can be closed. But happy to re-open if anyone would like.Do you know if the current Pillow annotations are at least on par with typeshed's ? As soon as Pillow is released with a
py.typedmarker, typeshed's stubs will be marked as obsolete. We'd then stop development ontypes-Pillowand remove it from typeshed's repo 6 months later.
It'd be unfortunate for users to loose on existing annotations, if other maintainers are left to believe thattypes-Pillowdoesn't need to be updated anymore. But if needed, the typeshed stubs can also be kept and made partial, to fill in the gaps in the mean time. Hence I'm asking.I checked typeshed a bit near the start, but I think on the whole we've added the hints from scratch. From a quick spot comparison, the coverage looks pretty good here.
Six months aligns nicely with two Pillow quarterly releases, which is a good time to fill in any important gaps. If you find some, we're happy for issues and PRs here. And I'd be fine with keeping the typeshed stubs around a bit longer if needed.
Reacted by Avasam, Neil Girdhar and Alex Waygood@hugovk Does closing this mean we now support type hints in enough of the code to declare that we support type hints? Just curious if we'll ever reach 100%, as I'm reviewing all the type hints PRs continuing to getting merged. Thanks for any info!
We declared that we support type hints in #7822. mypy gives us a pass, at least.
However, #8029 has been opened, questioning whether we should have done that before reaching 100%.
Ah! Thanks @radarhere . In that case, I'd maybe consider reopening this one and making the new standard for closing it "We're done adding type hints for now". Either way thanks again for the explanation. 👍
Yeah, we maybe added
py.typedand the classifier a bit early (#2625 (comment)), but I think we can use #8029 to track current progress. The definition of 100% also depends on the mypy settings used -- what are the "correct" settings? We could consider removingpy.typedfor the next quarterly release, but I welcome PRs from Pillow users who want more hints for certain files :)Reacted by Jonathan TranReacted by Jeffrey 'Alex' Clark#8029 has now been resolved, meaning that all of our methods are now typed.
disallow_untyped_defs = truehas been added to pyproject.toml to prevent untyped methods from reappearing in the future.Reacted by Hugo van Kemenade, Neil Girdhar and Colin Dean
I've been looking at applying typing to some code which uses PIL/pillow.
Is there any feeling regarding the addition of type annotations within Pillow, or if these belong outside the code in eg. typeshed? This influences whether I might work on a PR against Pillow itself, or generate stubs for typeshed.
On the basis of supporting python 2 and 3, I expect annotations would be in comments, and there would be an import of the typing module near the top, to define required types.