Accurate overloads for `ZipFile.init` #12119

max-muoto · 2024-06-08T21:32:48Z

Add overload cases based on each mode to eliminate false positives for types that don't fully implement IO[bytes], per #10880

max-muoto · 2024-06-08T21:50:55Z

Run-through of MyPy primer here:

Pip:

Removes false positive as the object they're passing in is readable, but doesn't implement IO[bytes]. They put a typing ignore, which is why this adds an additional warning.

werkzeug:

Removes false positive in test where a writable ResponseStream object is being passed into ZipFile.__init__, as the mode is set to "w".

sobolevn

I would consider adding test cases for this overload. It is rather complex.

max-muoto · 2024-06-09T07:23:20Z

I would consider adding test cases for this overload. It is rather complex.

Sounds good, can handle that tomorrow!

max-muoto · 2024-06-09T17:15:10Z

I would consider adding test cases for this overload. It is rather complex.

Added tests, found some minor issues which I fixed as a result: 4cf07ab

stdlib/zipfile/__init__.pyi

max-muoto · 2024-07-04T04:36:47Z

@JelleZijlstra Just wanted to follow up on this one!

JelleZijlstra · 2024-07-04T05:10:53Z

I'll periodically look over open typeshed PRs when I have some time, but can't guarantee I'll come back to a specific one (and other maintainers may also pick up this PR if they're confident enough in the change to merge it).

max-muoto · 2024-07-04T05:11:49Z

I'll periodically look over open typeshed PRs when I have some time, but can't guarantee I'll come back to a specific one (and other maintainers may also pick up this PR if they're confident enough in the change to merge it).

Sounds good, thanks for the initial look nonetheless.

hauntsaninja · 2024-07-05T20:20:04Z

stdlib/zipfile/__init__.pyi

+        @overload
+        def __init__(
+            self,
+            file: StrPath | _ZipReadable | _ZipSeekTellable,


Is this right? I'm not sure _ZipSeekTellable is enough, e.g. _RealGetContents requires read

You're right, we want a protocol that is seekable/readable if it's a valid zip file, otherwise it should implement tell: 0b52c6a

I think likely misread this, and assumed if an attribute error occurred it just needed to be tellable.

srittau · 2024-07-12T12:27:46Z

stdlib/zipfile/__init__.pyi

        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: ...


Aren't all following overloads shadowed by this one?

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.

As for _ZipFileMode, if file doesn't match we'll end up skipping over the overload. The tests show that this is working properly.

IO[bytes] is broader than _ZipReadable

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.

I've opened python/typing#1803 for clarification.

How is that a problem though? _ZipReadable will get matched if IO[bytes] isn't fully fulfilled, or am I misunderstanding something?

hauntsaninja

Thanks, this looks good, but had one more question!

hauntsaninja · 2024-08-10T22:58:25Z

stdlib/zipfile/__init__.pyi

        ) -> None: ...
        @overload
        def __init__(
            self,
-            file: StrPath | IO[bytes],
-            mode: _ZipFileMode = "r",
+            file: StrPath | _ZipTellable | _ZipWritable,


While _ZipTellable won't immediately error, I don't think the resulting object can be used for anything right? Unless I'm missing some use case, I think we should keep it StrPath | _ZipWritable

Oh in fact it's definitely wrong, because __del__ will trigger close will trigger a write attempt

Let me fix this.

@hauntsaninja Should be resolved now.

github-actions · 2024-08-11T18:13:13Z

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

pip (https://github.com/pypa/pip)
+ src/pip/_internal/network/lazy_wheel.py:162: error: Unused "type: ignore" comment  [unused-ignore]

werkzeug (https://github.com/pallets/werkzeug)
- tests/test_wrappers.py:1216: error: No overload variant of "ZipFile" matches argument types "ResponseStream", "str"  [call-overload]
- tests/test_wrappers.py:1216: note: Possible overload variants:
- tests/test_wrappers.py:1216: note:     def __init__(self, file: str | PathLike[str] | IO[bytes], mode: Literal['r'] = ..., compression: int = ..., allowZip64: bool = ..., compresslevel: int | None = ..., *, strict_timestamps: bool = ..., metadata_encoding: str | None) -> ZipFile
- tests/test_wrappers.py:1216: note:     def __init__(self, file: str | PathLike[str] | IO[bytes], mode: Literal['r', 'w', 'x', 'a'] = ..., compression: int = ..., allowZip64: bool = ..., compresslevel: int | None = ..., *, strict_timestamps: bool = ..., metadata_encoding: None = ...) -> ZipFile

hauntsaninja · 2024-08-11T23:16:15Z

Thank you!

Co-authored-by: Shantanu <[email protected]>

max-muoto added 2 commits June 8, 2024 16:32

chore: Accurate overloads for ZipFile.__init__

c2505eb

Tweak

6eafa4e

max-muoto marked this pull request as ready for review June 8, 2024 21:39

max-muoto changed the title ~~chore: Accurate overloads for ZipFile.__init__~~ Accurate overloads for ZipFile.__init__ Jun 8, 2024

Tweak

6bc28bd

This comment has been minimized.

Sign in to view

sobolevn reviewed Jun 9, 2024

View reviewed changes

max-muoto and others added 2 commits June 9, 2024 12:13

Add test

4cf07ab

[pre-commit.ci] auto fixes from pre-commit.com hooks

7f75295

max-muoto and others added 3 commits June 9, 2024 12:16

Tweaks

c9704b9

Tweak

377ce76

Merge branch 'main' into accurate-zip-overloads

aca61a7

This comment has been minimized.

Sign in to view

max-muoto and others added 3 commits June 9, 2024 12:29

Add missing annotations

3af922f

Missing annotation

0f7c9e0

[pre-commit.ci] auto fixes from pre-commit.com hooks

d82e36f

This comment has been minimized.

Sign in to view

JelleZijlstra reviewed Jun 20, 2024

View reviewed changes

stdlib/zipfile/__init__.pyi Outdated Show resolved Hide resolved

Use positional arguments

97a4a95

This comment has been minimized.

Sign in to view

Merge branch 'main' into accurate-zip-overloads

a682d36

This comment has been minimized.

Sign in to view

hauntsaninja reviewed Jul 5, 2024

View reviewed changes

Address feedback

0b52c6a

max-muoto added 2 commits July 5, 2024 16:17

Shorten name

814ab6b

Fix tests

524d7d7

This comment has been minimized.

Sign in to view

srittau reviewed Jul 12, 2024

View reviewed changes

Merge branch 'main' into accurate-zip-overloads

56d44c0

This comment has been minimized.

Sign in to view

hauntsaninja approved these changes Aug 10, 2024

View reviewed changes

max-muoto added 2 commits August 11, 2024 13:04

Address feedback

d98196a

Fix

cc520e2

hauntsaninja merged commit dc0b63f into python:main Aug 11, 2024
63 checks passed

max-muoto added a commit to max-muoto/typeshed that referenced this pull request Aug 17, 2024

Accurate overloads for ZipFile.__init__ (python#12119)

bfaac35

Co-authored-by: Shantanu <[email protected]>

max-muoto added a commit to max-muoto/typeshed that referenced this pull request Sep 8, 2024

Accurate overloads for ZipFile.__init__ (python#12119)

ea5e2b1

Co-authored-by: Shantanu <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accurate overloads for `ZipFile.init` #12119

Accurate overloads for `ZipFile.init` #12119

max-muoto commented Jun 8, 2024 •

edited

Loading

This comment has been minimized.

max-muoto commented Jun 8, 2024 •

edited

Loading

sobolevn left a comment

max-muoto commented Jun 9, 2024

max-muoto commented Jun 9, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

max-muoto commented Jul 4, 2024

JelleZijlstra commented Jul 4, 2024

max-muoto commented Jul 4, 2024

hauntsaninja Jul 5, 2024

max-muoto Jul 5, 2024 •

edited

Loading

max-muoto Jul 5, 2024

This comment has been minimized.

srittau Jul 12, 2024

max-muoto Jul 13, 2024

max-muoto Jul 13, 2024 •

edited

Loading

srittau Jul 15, 2024

srittau Jul 15, 2024

max-muoto Jul 15, 2024

This comment has been minimized.

hauntsaninja left a comment

hauntsaninja Aug 10, 2024

hauntsaninja Aug 10, 2024

max-muoto Aug 10, 2024

max-muoto Aug 11, 2024

max-muoto Aug 11, 2024

github-actions bot commented Aug 11, 2024

hauntsaninja commented Aug 11, 2024

Accurate overloads for ZipFile.__init__ #12119

Accurate overloads for ZipFile.__init__ #12119

Conversation

max-muoto commented Jun 8, 2024 • edited Loading

This comment has been minimized.

max-muoto commented Jun 8, 2024 • edited Loading

sobolevn left a comment

Choose a reason for hiding this comment

max-muoto commented Jun 9, 2024

max-muoto commented Jun 9, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

max-muoto commented Jul 4, 2024

JelleZijlstra commented Jul 4, 2024

max-muoto commented Jul 4, 2024

Choose a reason for hiding this comment

max-muoto Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-muoto Jul 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

hauntsaninja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 11, 2024

hauntsaninja commented Aug 11, 2024

Accurate overloads for `ZipFile.init` #12119

Accurate overloads for `ZipFile.init` #12119

max-muoto commented Jun 8, 2024 •

edited

Loading

max-muoto commented Jun 8, 2024 •

edited

Loading

max-muoto Jul 5, 2024 •

edited

Loading

max-muoto Jul 13, 2024 •

edited

Loading