Closed PeterJCLaw closed 4 years ago
Would there be interest in adding type annotations within jedi?
Yes, but please not with Python 2.7. I'm going to drop 2.7 in a month or two. So I doubt that it would make sense until that happens :).
There are definitely some patterns within jedi which aren't immediately obvious how to type
Fully agreed.
I'm trying to get rid of duck typing more and more. So it shouldn't be that crazy. Mostly you could argue that a ValueSet
holds Value
s.
All right. I'm finally done with the Python 3 branch. It's not that I want to release this in a week, but at least now we can add annotations.
In general where do you want to add annotations? I'm pretty open to having them anywhere, as long as it's not like inference_state: InferenState
everywhere. But in a lot of places it makes a lot of sense and I have also started adding them (to distinguish stuff like Path
from str
and similar cases). If you're in the same boat, I will just close this issue and you can start adding them wherever you want :).
I'm probably going to do one more release (0.17.2?) for Python 2. The next release should be Python 3 after that (0.18.0?) and if that's working well I will release 1.0.0.
In most of my projects I'm a big fan of having them everywhere -- really leaning into the type system to clarify what things are and making the type checker work to catch issues by using a really strict configuration. I know it can feel a bit redundant/overkill to annotate basic things, however from experience it is usually worth it in the long run. Obviously that's a lot easier with small or new projects, though it's also possible to have a config which is strict where the annotations are present but lax elsewhere (which is what we have at work, where we have a large existing Python codebase which is slowly gathering annotations).
For Jedi I think it's definitely worth having complete coverage over the places where the API is considered public (and publishing that jedi has annotations) so that anyone integrating against jedi can be sure about their usage of the API if they want to.
As noted previously, the internals are going to be a bit harder to annotate in a strict manner without code changes, so we probably want a different approach. For the internals I think it's worth working out what we want from the annotations. The benefits I've found I get from them (not all of which I'd expected) are:
inference_state
is an InferenceState
and roughly where to go to work out what one of those is, it definitely wasn't obvious when I first started looking through the source. As a new reader, you don't know if it's a specific type, a conventional shape of mapping/etc. or something else. In that regard, having type annotations is like having interactive docstrings which are (mostly) guaranteed to be up to date.jedi
itself will be able to pick up on the annotations it'll result in better editor completions anywhere that is annotated, which will accelerate all contributors. This is the benefit I most value from annotations day-to-day and the reason that I've started checking whether a project has annotations before I start using it.NoneType has no attribute 'x'
. While this is the headline feature of type annotations, it's strangely rare that I find this catching real issues. There's definitely lots of ways that the type checkers end up being stricter than a human would consider is necessary, which for projects where I'm the only author I do find occasionally frustrating, however when working with others it removes another set of things I need to bother checking at code review -- if the type checker says it works I don't need to worry about None
checks and the like.In order to get all of those benefits however you do need to annotate almost everything, including the boilerplate.
Regardless of what we decide regarding the actual annotations (and maybe different policies in different areas of the codebase would be useful), I suggest that we aim to get in place a minimal type checker config (perhaps with no actual annotations) which is running on CI before closing this issue.
I suggest that we aim to get in place a minimal type checker config
Feel free to implement that :) I'm not much of an expert with Mypy and others.
For Jedi I think it's definitely worth having complete coverage over the places where the API is considered public (and publishing that jedi has annotations) so that anyone integrating against jedi can be sure about their usage of the API if they want to.
+1. We should probably just remove the docstring type annotations at the same time. A lot of the public API is already "annotated", just not with proper annotations.
For the internals I think it's worth working out what we want from the annotations. The benefits I've found I get from them (not all of which I'd expected) are:
I'm not against adding annotations at all. To me the main benefit of annotations is readabiliity. So if it adds a lot of clutter and we need a lot of cast
and stuff, it's just harder to read. However I have already started some annotations, so there's definitely a sweet spot IMO.
I think you should probably just start annotating and then we can discuss anything more in PRs. (please use the python3
branch).
I am interested in helping out with this.
I propose though, that we do this incrementally. Having a big PR with all type annotations added at once might be overwhelming. What do you think? Incremental adoption would also allow us to parallelize the work.
I will also not merge a big PR. Annotating the API is of course a good idea and should probably be done first. People can take it from there.
BTW: I'm not interested in perfect mypy coverage. I'm rather interested in good IDE completions.
Agree on this being very incremental. My rough plan was to have the first PR just be the local & CI config, plus whatever minimal annotations are needed to make that pass.
I'd probably go for mypy
, though only because it's the checker I have experience with.
I'd also probably have a config which starts off strict where annotations are present, but otherwise doesn't check. It's easier to relax (or ignore) a config which is strict than it is to later make it strict.
I may have some time this week to look at this.
Once the basics are in place I'd be tempted to run MonkeyType to ramp up the coverage quickly.
I've put up https://github.com/davidhalter/jedi/pull/1642, which I've tagged as closing this issue. I suggest that once that PR merges we create separate issues for each of the themes of annotations we've discussed as being desirable and let this issue close.
Would there be interest in adding type annotations within jedi?
I realise the codebase currently supports Python 2.7 still, which would complicate things a bit, though I think this could help both catch some errors and also make it a bit easier for newcomers to navigate the code.
There are definitely some patterns within jedi which aren't immediately obvious how to type (
ValueSet
is going to be very interesting), but I still think there could be some value in this.