-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add type annotations to _pytest.config.argparsing and corresponding Config code #6241
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
Conversation
This feature was added in commit 007a77c, but was never used in pytest itself. A GitHub code search doesn't find any users either (only pytest repo copies). It seems safe to clean up.
@@ -61,7 +76,7 @@ def getgroup(self, name, description="", after=None): | |||
self._groups.insert(i + 1, group) | |||
return group | |||
|
|||
def addoption(self, *opts, **attrs): | |||
def addoption(self, *opts: str, **attrs: Any) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if attrs
could be typed. Would be the same as with Argument.__init__
's attrs
then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, TypeScript has this feature, but mypy doesn't AFAIK. The other option is to duplicate, but I wouldn't want to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could a typevar be used here, to be used in those (3+) places?
But this could also be done later anyway - I've just thought it would be really helpful with completion of those functions.
(You have probably noticed also that documentation in this regard is outdated etc, and it's messy in general, so that would help there.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could a typevar be used here, to be used in those (3+) places?
Not that I know of...
e3e2333
to
b6682c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
b6682c8
to
dac16cd
Compare
@blueyed I missed a couple of places that can take a |
How does that behave by now? IIRC it's handled as I think it makes sense therefore to type |
It's still
That would be good, it does show up everywhere in pytest. I'm not entirely sure we should add it in pytest - what would happen when we finally publish our types and a user encounters |
The first commit cleans up some old feature that slightly interfered with the types. The second commit adds annotations to the argument parsing code.