Type annotate main.py and some parts related to collection #6556
Type annotate main.py and some parts related to collection #6556bluetech wants to merge 1 commit intopytest-dev:masterfrom
Conversation
bluetech
left a comment
There was a problem hiding this comment.
Left comments on where I needed to make code changes, for greater scrutiny.
| # Module itself, so just use that. If this special case isn't taken, then all | ||
| # the files in the package will be yielded. | ||
| if argpath.basename == "__init__.py": | ||
| assert isinstance(m[0], nodes.Collector) |
There was a problem hiding this comment.
Needed to add an assert here (can also be a nodes.Item), but seems safe given it should be a Package and .collect() is called just below.
| assert isinstance(m[0], nodes.Collector) | ||
| try: | ||
| yield next(m[0].collect()) | ||
| yield next(iter(m[0].collect())) |
There was a problem hiding this comment.
Needed to add iter() here because I made the official type of collect() be Iterable and not Iterator (which isn't the case for all implementations). Calling iter() on an iterator should just be idempotent, so I think it's safe.
| except (AttributeError, ImportError, ValueError): | ||
| return x | ||
| if spec is None or spec.origin in {None, "namespace"}: | ||
| if spec is None or spec.origin is None or spec.origin == "namespace": |
There was a problem hiding this comment.
Had to split it because mypy doesn't handle in {None} for type narrowing.
| if not safe_getattr(self.obj, "__test__", True): | ||
| return [] | ||
| if hasinit(self.obj): | ||
| assert self.parent is not None |
There was a problem hiding this comment.
This assert and below should be safe since dereferenced just below.
| _setupstate = None # type: SetupState | ||
| # Set on the session by fixtures.pytest_sessionstart. | ||
| _fixturemanager = None # type: FixtureManager | ||
| exitstatus = None # type: Union[int, ExitCode] |
There was a problem hiding this comment.
Didn't find any place which does hasattr('exitstatus') or such, so I think this is safe.
There was a problem hiding this comment.
It's still kind of an API change.
But I also think it is good to have it being set always.
| # we are inside a make_report hook so | ||
| # we cannot directly pass through the exception | ||
| self._notfound.append((report_arg, sys.exc_info()[1])) | ||
| exc = cast(NoMatch, sys.exc_info()[1]) |
There was a problem hiding this comment.
I don't really follow the comment above. I think this cast is correct...
| _setupstate = None # type: SetupState | ||
| # Set on the session by fixtures.pytest_sessionstart. | ||
| _fixturemanager = None # type: FixtureManager | ||
| exitstatus = None # type: Union[int, ExitCode] |
There was a problem hiding this comment.
It's still kind of an API change.
But I also think it is good to have it being set always.
This comment has been minimized.
This comment has been minimized.
5c1256f to
49f830d
Compare
|
Addressed (some) comments and rebased on features. This is still blocked. |
Taken out of pytest-dev#6556.
Taken out of pytest-dev#6556.
Taken out of pytest-dev#6556.
49f830d to
eb648ab
Compare
Taken out of pytest-dev#6556.
Taken out of pytest-dev#6556.
Pulled out of pytest-dev#6556.
Pulled out of pytest-dev#6556.
eb648ab to
8430b21
Compare
This comment has been minimized.
This comment has been minimized.
f1e712f to
bde54d1
Compare
|
Rebased on |
bde54d1 to
abdd513
Compare
|
Maintaining all of these open PRs is too annoying and spammy. I'm going to close them and maintain just a single |
Depends on #6547 and #6555 (and includes them until merged).This adds type annotations to main.py. To do this, I needed to feel in some blanks in other parts, mostly related to collection.
It took some effort to untangle but I think I got it mostly right...