-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Accurate overloads for ZipFile.__init__
#12119
Changes from 13 commits
c2505eb
6eafa4e
6bc28bd
4cf07ab
7f75295
c9704b9
377ce76
aca61a7
3af922f
0f7c9e0
d82e36f
97a4a95
a682d36
0b52c6a
814ab6b
524d7d7
56d44c0
d98196a
cc520e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
from __future__ import annotations | ||
|
||
import io | ||
import pathlib | ||
import zipfile | ||
from typing import Literal | ||
|
||
### | ||
# Tests for `zipfile.ZipFile` | ||
### | ||
|
||
p = pathlib.Path("test.zip") | ||
|
||
|
||
class CustomPathObj: | ||
def __init__(self, path: str) -> None: | ||
self.path = path | ||
|
||
def __fspath__(self) -> str: | ||
return self.path | ||
|
||
|
||
class NonPathObj: | ||
def __init__(self, path: str) -> None: | ||
self.path = path | ||
|
||
|
||
class ReadableObj: | ||
def seek(self, offset: int, whence: int = 0) -> int: | ||
return 0 | ||
|
||
def read(self, n: int | None = -1) -> bytes: | ||
return b"test" | ||
|
||
|
||
class TellableObj: | ||
def tell(self) -> int: | ||
return 0 | ||
|
||
|
||
class WriteableObj: | ||
def close(self) -> None: | ||
pass | ||
|
||
def write(self, b: bytes) -> int: | ||
return len(b) | ||
|
||
def flush(self) -> None: | ||
pass | ||
|
||
|
||
class SeekTellObj: | ||
def seek(self, offset: int, whence: int = 0) -> int: | ||
return 0 | ||
|
||
def tell(self) -> int: | ||
return 0 | ||
|
||
|
||
def write_zip(mode: Literal["r", "w", "x", "a"]) -> None: | ||
# Test any mode with `pathlib.Path` | ||
with zipfile.ZipFile(p, mode) as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Test any mode with `str` path | ||
with zipfile.ZipFile("test.zip", mode) as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Test any mode with `os.PathLike` object | ||
with zipfile.ZipFile(CustomPathObj("test.zip"), mode) as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Non-PathLike object should raise an error | ||
with zipfile.ZipFile(NonPathObj("test.zip"), mode) as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# IO[bytes] like-obj should work for any mode. | ||
io_obj = io.BytesIO(b"This is a test") | ||
with zipfile.ZipFile(io_obj, mode) as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Readable object should not work for any mode. | ||
with zipfile.ZipFile(ReadableObj(), mode) as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# Readable object should work for "r" mode. | ||
with zipfile.ZipFile(ReadableObj(), "r") as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Readable object should work for "a" mode. | ||
with zipfile.ZipFile(ReadableObj(), "a") as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Readable object should not work for "w" mode. | ||
with zipfile.ZipFile(ReadableObj(), "w") as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# Tellable object should not work for any mode. | ||
with zipfile.ZipFile(TellableObj(), mode) as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# Tellable object should work for "w" mode. | ||
with zipfile.ZipFile(TellableObj(), "w") as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Writeable object should not work for any mode. | ||
with zipfile.ZipFile(WriteableObj(), mode) as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# Writeable object should work for "w" mode. | ||
with zipfile.ZipFile(WriteableObj(), "w") as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Seekable and Tellable object should not work for any mode. | ||
with zipfile.ZipFile(SeekTellObj(), mode) as z: # type: ignore | ||
z.writestr("test.txt", "test") | ||
|
||
# Seekable and Tellable object should work for "w" mode. | ||
with zipfile.ZipFile(SeekTellObj(), "w") as z: | ||
z.writestr("test.txt", "test") | ||
|
||
# Seekable and Tellable object should work for "a" mode. | ||
with zipfile.ZipFile(SeekTellObj(), "a") as z: | ||
z.writestr("test.txt", "test") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,6 +94,22 @@ class ZipExtFile(io.BufferedIOBase): | |
class _Writer(Protocol): | ||
def write(self, s: str, /) -> object: ... | ||
|
||
class _ZipReadable(Protocol): | ||
def seek(self, offset: int, whence: int = 0, /) -> int: ... | ||
def read(self, n: int = -1, /) -> bytes: ... | ||
|
||
class _ZipTellable(Protocol): | ||
def tell(self) -> int: ... | ||
|
||
class _ZipSeekTellable(Protocol): | ||
def seek(self, offset: int, whence: int = 0, /) -> int: ... | ||
def tell(self) -> int: ... | ||
|
||
class _ZipWritable(Protocol): | ||
def flush(self) -> None: ... | ||
def close(self) -> None: ... | ||
def write(self, b: bytes, /) -> int: ... | ||
|
||
class ZipFile: | ||
filename: str | None | ||
debug: int | ||
|
@@ -106,24 +122,50 @@ class ZipFile: | |
compresslevel: int | None # undocumented | ||
mode: _ZipFileMode # undocumented | ||
pwd: bytes | None # undocumented | ||
# metadata_encoding is new in 3.11 | ||
if sys.version_info >= (3, 11): | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | IO[bytes], | ||
mode: _ZipFileMode = "r", | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
metadata_encoding: str | None = None, | ||
) -> None: ... | ||
# metadata_encoding is only allowed for read mode | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | _ZipReadable, | ||
mode: Literal["r"] = "r", | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
metadata_encoding: str | None, | ||
metadata_encoding: str | None = None, | ||
) -> None: ... | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | IO[bytes], | ||
mode: _ZipFileMode = "r", | ||
file: StrPath | _ZipTellable | _ZipWritable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh in fact it's definitely wrong, because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me fix this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hauntsaninja Should be resolved now. |
||
mode: Literal["w", "x"] = ..., | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
metadata_encoding: None = None, | ||
) -> None: ... | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | _ZipReadable | _ZipSeekTellable, | ||
mode: Literal["a"] = ..., | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
|
@@ -132,6 +174,7 @@ class ZipFile: | |
metadata_encoding: None = None, | ||
) -> None: ... | ||
else: | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | IO[bytes], | ||
|
@@ -142,6 +185,39 @@ class ZipFile: | |
*, | ||
strict_timestamps: bool = True, | ||
) -> None: ... | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | _ZipReadable, | ||
mode: Literal["r"] = "r", | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
) -> None: ... | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | _ZipTellable | _ZipWritable, | ||
mode: Literal["w", "x"] = ..., | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
) -> None: ... | ||
@overload | ||
def __init__( | ||
self, | ||
file: StrPath | _ZipReadable | _ZipSeekTellable, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this right? I'm not sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, we want a protocol that is seekable/readable if it's a valid zip file, otherwise it should implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think likely misread this, and assumed if an attribute error occurred it just needed to be tellable. |
||
mode: Literal["a"] = ..., | ||
compression: int = 0, | ||
allowZip64: bool = True, | ||
compresslevel: int | None = None, | ||
*, | ||
strict_timestamps: bool = True, | ||
) -> None: ... | ||
|
||
def __enter__(self) -> Self: ... | ||
def __exit__( | ||
|
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.
Aren't all following overloads shadowed by this one?
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.
No - for example,
IO[bytes]
is broader than_ZipReadable
, which will lead to the second overload getting chosen if you just an object that is readable.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.
As for
_ZipFileMode
, iffile
doesn't match we'll end up skipping over the overload. The tests show that this is working properly.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.
But that's exactly the problem. It's my understanding that overloads are processed in order (although the typing spec doesn't mention that), so
IO[bytes]
always matches before_ZipReadable
can match.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've opened python/typing#1803 for clarification.
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.
How is that a problem though?
_ZipReadable
will get matched ifIO[bytes]
isn't fully fulfilled, or am I misunderstanding something?