Add stubs for docopt package
#5248
Conversation
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
Please review and let me know what needs improvement, this is my first time getting into types and typeshed! For instance, I didn't understand how your tests work (I ran them, but wasn't exactly sure what they did), and I wasn't sure how to actually verify that these definitions are compatible with the actual Thanks! |
Don't worry too much about tests. We don't have unit tests for everything, and that's intentional. See #1339 for discussion. If you know an open-source project that uses mypy and docopt, you can add it to mypy_primer. It runs for all typeshed pull requests, and that way we can be sure that pull requests don't break the Personally, I don't run most of typeshed's tests locally, because they run on pull requests anyway. |
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
Nit: If possible, don't |
Add the single stub for the simple, but fairly well-used [docopt](https://github.com/docopt/docopt) package. It looks like that project is not very well maintained, so I thought it would be easier to add the type annotations here, as opposed to there, upstream. This has even been requested a few times in docopt/docopt#471. It appeared to me that only the `docopt()` function is the public API, so I didn't include the rest of the module's functions and attributes. I believe that this stub is totally correct, except for one edge case: In https://github.com/docopt/docopt/blob/20b9c4ffec71d17cee9fd963238c8ec240905b65/docopt.py#L569 you can see that the `argv` argument can not only be a an `Iterable[str]`, but `argv` could also be a type which supports a `split()` function: ``` from typing_extensions import Protocol class _SplittableToStr(Protocol): def split(self, *args: Any, **kwargs: Any) -> Iterable[str]: ... ``` However, I'm not sure I got that correct, and I decided (it's a guess) that not many users would use anything but a `list[str]` for `argv`, so I just kept it simple and didn't include that Protocol stuff.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Oops, sorry, just did that again. Also makes sense. I'll keep that in mind for next time! |
(I haven't read #1339 yet) Won't by definition there not be any projects that use mypy and docopt, since docopt doesn't have types? Or does it work for projects that have their own |
|
It seems that most people just |
|
I found this with Github's code search. |
There was a problem hiding this comment.
I just looked at somebody's fork of docopt that contains hints and thought maybe we should borrow some of their ideas. I left a review on this PR with specifics.
I found this with Github's code search.
Gotcha, that's neat. That project looks tiny/unmaintained, but I see 44 other projects that might be better candidates for mypy_primer, I'll investigate them.
stubs/docopt/docopt.pyi
Outdated
|
|
||
| def docopt( | ||
| doc: str, | ||
| argv: Optional[Iterable[str]] = ..., |
There was a problem hiding this comment.
Maybe this could be made slightly more accepting (re: the split() method mentioned in the Commit message) if this was Optional[Union[Iterable[str], str]]?
There was a problem hiding this comment.
Yes, that would be better. I suggested a type equivalent to that earlier.
There was a problem hiding this comment.
While Optional[Iterable[str]] is technically correct, it doesn't make it obvious that the hasattr(thing, 'split') was considered when writing the stubs. That's why I preferred to be explicit.
In some cases, mypy specifically disallows relying on that (a, b = some_string doesn't work, for example), but I'm not sure if that applies here.
There was a problem hiding this comment.
@JelleZijlstra indeed that's true, but do you think that maybe is a bit too clever? I think listing them out separately might be more intuitive to people, e.g. ["--time", "5"] as well as "--time 5" both work?
It can now be a `str` in addition to being a `Iterable[str]`.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
FYI, mypy_primer has been broken for third party stubs since modular typeshed. I should get around to fixing it. |
|
Thanks @Akuli, @hauntsaninja @JelleZijlstra 😄! |
Add the single stub for the simple, but fairly well-used
docopt package.
It looks like that project is not very well maintained,
so I thought it would be easier to add the type annotations
here, as opposed to there, upstream. This has even been requested a few
times in docopt/docopt#471. It appeared to me
that only the
docopt()function is the public API, so I didn't includethe rest of the module's functions and attributes.
I believe that this stub is totally correct, except for one edge case: In https://github.com/docopt/docopt/blob/20b9c4ffec71d17cee9fd963238c8ec240905b65/docopt.py#L569
you can see that the
argvargument can not only be a list, butargvcould also be a type which supports a
split()function:However, I'm not sure I got that correct, and I decided (it's a guess)
that not many users would use anything but a
list[str]forargv, soI just kept it simple and didn't include that Protocol stuff.