tarfile.open(): Handle all modes

[srittau]

No description provided.

[AlexWaygood]

Looks like you're running into python/mypy#16367. It's been fixed on mypy master, but the fix hasn't yet been included in a mypy release.

[AlexWaygood]

Looks like you're running into python/mypy#16367. It's been fixed on mypy master, but the fix hasn't yet been included in a mypy release.

Oh sorry, you already noted as such in #12182 (comment) :-)

[srittau]

Looks like you're running into python/mypy#16367. It's been fixed on mypy master, but the fix hasn't yet been included in a mypy release.

Oh sorry, you already noted as such in #12182 (comment) :-)

I sneak edited it in when I noticed the problem. (Which had already been noted by @max-muoto in #12117.)

[srittau]

vision is doing some dynamic construction of the mode string. This should be easily solvable by using literals and an intermediate variable. Which sounds like a good idea to ensure that no invalid mode strings are constructed.

pip is assigning the mode string using an if/elif cascade. This could also be solved by using an intermediate variable.

Both are not great, but I'm personally fine with these, given the increases soundness of the annotations. (Of course, in pip's case it would be even better if mypy would automatically infer that mode is a literal with certain values.)

[JelleZijlstra]

This has a merge conflict.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

vision (https://github.com/pytorch/vision)
+ torchvision/datasets/utils.py:219: error: No overload variant of "open" matches argument types "str | Path", "str"  [call-overload]
+ torchvision/datasets/utils.py:219: note: Possible overload variants:
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., mode: Literal['r', 'r:*', 'r:', 'r:gz', 'r:bz2', 'r:xz'] = ..., fileobj: IO[bytes] | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x', 'x:', 'a', 'a:', 'w', 'w:'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x', 'x:', 'a', 'a:', 'w', 'w:'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x:gz', 'x:bz2', 'w:gz', 'w:bz2'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., compresslevel: int = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x:gz', 'x:bz2', 'w:gz', 'w:bz2'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., compresslevel: int = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x:xz', 'w:xz'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: Literal[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | None = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x:xz', 'w:xz'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: Literal[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | None = ...) -> TarFile
+ torchvision/datasets/utils.py:219: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: str, fileobj: IO[bytes] | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: int | None = ...) -> TarFile

pip (https://github.com/pypa/pip)
+ src/pip/_internal/utils/unpacking.py:179: error: No overload variant of "open" matches argument types "str", "str", "str"  [call-overload]
+ src/pip/_internal/utils/unpacking.py:179: note: Possible overload variants:
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., mode: Literal['r', 'r:*', 'r:', 'r:gz', 'r:bz2', 'r:xz'] = ..., fileobj: IO[bytes] | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x', 'x:', 'a', 'a:', 'w', 'w:'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x', 'x:', 'a', 'a:', 'w', 'w:'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x:gz', 'x:bz2', 'w:gz', 'w:bz2'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., compresslevel: int = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x:gz', 'x:bz2', 'w:gz', 'w:bz2'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., compresslevel: int = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None, mode: Literal['x:xz', 'w:xz'], fileobj: _Fileobj | None = ..., bufsize: int = ..., *, format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: Literal[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | None = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: Literal['x:xz', 'w:xz'], fileobj: _Fileobj | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: Literal[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] | None = ...) -> TarFile
+ src/pip/_internal/utils/unpacking.py:179: note:     def open(name: str | bytes | PathLike[str] | PathLike[bytes] | None = ..., *, mode: str, fileobj: IO[bytes] | None = ..., bufsize: int = ..., format: int | None = ..., tarinfo: type[TarInfo] | None = ..., dereference: bool | None = ..., ignore_zeros: bool | None = ..., encoding: str | None = ..., errors: str = ..., pax_headers: Mapping[str, str] | None = ..., debug: int | None = ..., errorlevel: int | None = ..., preset: int | None = ...) -> TarFile

[JelleZijlstra]

I see this argument being used in the implementation, why did you remove it?

[srittau]

Due to this comment from the docs:

For modes 'w:gz', 'x:gz', 'w|gz', 'w:bz2', 'x:bz2', 'w|bz2', tarfile.open() accepts the keyword argument compresslevel (default 9) to specify the compression level of the file.