Change scandir() to type as both iterator and context-manager #1583
Conversation
Summary: The existing typing for scandir() assumes it can only be an iterator. It can also be used as a context manager per the documentation, and typing fails when used as part of mypy-0.530. Hat-tip to a colleague, M. Pieters, for the guidance on how to do this. Test Plan: Wrote a small snippet of code that implements the context manager usage. Ran stock mypy against that code, got warnings about exit and enter. Ran amended mypy (ie, with this typing included) against that code, got no warnings.
| def __next__(self) -> DirEntry[str]: ... | ||
| def __enter__(self: _T) -> _T: ... | ||
| def __exit__(self, exc_type: Optional[type], exc_val: Optional[Exception], | ||
| exc_tb: Optional[TracebackType]) -> bool: ... | ||
| @overload | ||
| def scandir() -> Iterator[DirEntry[str]]: ... |
There was a problem hiding this comment.
Why is this overload unaffected?
stdlib/3/os/__init__.pyi
Outdated
| @@ -343,10 +344,16 @@ if sys.version_info >= (3, 3): | |||
| def replace(src: _PathType, dst: _PathType) -> None: ... | |||
| def rmdir(path: _PathType) -> None: ... | |||
| if sys.version_info >= (3, 6): | |||
| class ScandirIterator(Iterable): | |||
There was a problem hiding this comment.
This needs to be generic in AnyStr (so inherit from Iterable[AnyStr], and then change all the str below to AnyStr.
There was a problem hiding this comment.
Also, the name should be prefixed with _ because this name is not publicly exported in the os module.
| class ScandirIterator(Iterable): | ||
| def __iter__(self) -> Iterator[DirEntry[str]]: ... | ||
| def __next__(self) -> DirEntry[str]: ... | ||
| def __enter__(self: _T) -> _T: ... |
There was a problem hiding this comment.
Instead of _T, use a special type variable with bound=ScandirIterator[AnyStr].
| def __iter__(self) -> Iterator[DirEntry[str]]: ... | ||
| def __next__(self) -> DirEntry[str]: ... | ||
| def __enter__(self: _T) -> _T: ... | ||
| def __exit__(self, exc_type: Optional[type], exc_val: Optional[Exception], |
There was a problem hiding this comment.
exc_type should be Optional[Type[BaseException]] and exc_val should be Optional[BaseException].
|
Ack requested changes. I'm going off-grid for a week, will come back to this when I'm back from holiday. |
|
Any updates? |
Rework of python#1583. Fixes python#1573. See documentation in https://docs.python.org/3/library/os.html#os.scandir.
|
I reimplemented the change in #1787 since the author of this PR is AWOL. |
Rework of #1583. Fixes #1573. See documentation in https://docs.python.org/3/library/os.html#os.scandir.
Summary:
The existing typing for scandir() assumes it can only be an iterator. It can also be used as a context manager per the documentation, and typing fails when used as part of mypy-0.530.
Hat-tip to a colleague, M. Pieters, for the guidance on how to do this.
Filed as issue #1573
Test Plan:
Wrote a small snippet of code that implements the context manager usage.
Ran stock mypy against that code, got warnings about exit and enter.
Ran amended mypy (ie, with this typing included) against that code, got no warnings.
Snippet: