-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
typing: Session.__init__ #6525
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
typing: Session.__init__ #6525
Conversation
4ad2104
to
e08ac2d
Compare
src/_pytest/main.py
Outdated
@@ -383,7 +391,7 @@ class Session(nodes.FSCollector): | |||
# Set on the session by fixtures.pytest_sessionstart. | |||
_fixturemanager = None # type: FixtureManager | |||
|
|||
def __init__(self, config): | |||
def __init__(self, config) -> 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.
Out of curiosity, is considered good practice to annotate the return value of all functions even if they return 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.
It has to be for the function to be considered to be typed.
I've amended it to also type config
(the arg) explicitly, although done via base class.
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.
I see, thanks makes sense, it is the only annotation.
Now it brings the next question, is:
def __init__(self, config: Config):
enough for it to be considered typed?
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.
Yes (AFAIK), but might fail with strict options etc?! /cc @bluetech
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.
btw: reveal_type(var)
is very useful in that regard.
Pulled out of pytest-dev#6491.
e08ac2d
to
bd6ba3f
Compare
# Keep track of any collected nodes in here, so we don't duplicate fixtures | ||
self._node_cache = {} | ||
self._node_cache = {} # type: Dict[str, List[Node]] |
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.
Apparently sometimes the key can be tuple so the type is not exactly correct:
Lines 574 to 579 in ad02f6f
key = (type(x), x.fspath) | |
if key in self._node_cache: | |
yield self._node_cache[key] | |
else: | |
self._node_cache[key] = x | |
yield x |
I am working on some annotations and will include a fix for this, hopefully tomorrow.
Pulled out of #6491.