Python 3.14: PEP-784 compression.zstd #14129

Rogdham · 2025-05-24T11:05:31Z

Second PR about PEP-784 in stdlib (first PR):

Add type hints for _zstd
Add type hints for compression.zstd

Rogdham · 2025-05-24T12:19:50Z

@emmatyping: if you want to give try this out, or simply comment on the use of Mapping[CompressionParameter, int] and Mapping[DecompressionParameter, int] as type hints for the options parameter. Any feedback would be appreciated!

emmatyping · 2025-05-24T15:04:53Z

Note that I typed the options as Mapping[CompressionParameter, int] / Mapping[DecompressionParameter, int], while a Mapping[int, int] is technically accepted.

I think we should type that it accepts Mapping[int, int] as well. I consider that a supported usage.

stdlib/_zstd.pyi

Rogdham · 2025-05-29T07:41:01Z

Now that CPython 3.14.0 beta 2 is released, the CI is green and the PR ready for review!

srittau

Thanks, a few remark below. Also I notice that many defaults are ..., although they have basic defaults in the implementation. In these cases, we include the actual defaults, ... is only used for complicated cases.

srittau · 2025-05-30T14:47:43Z

stdlib/_zstd.pyi

+    CONTINUE: Final[_ZstdCompressorContinue] = 0
+    FLUSH_BLOCK: Final[_ZstdCompressorFlushBlock] = 1
+    FLUSH_FRAME: Final[_ZstdCompressorFlushFrame] = 2


The Final implies the Literal:

Suggested change

CONTINUE: Final[_ZstdCompressorContinue] = 0

FLUSH_BLOCK: Final[_ZstdCompressorFlushBlock] = 1

FLUSH_FRAME: Final[_ZstdCompressorFlushFrame] = 2

CONTINUE: Final = 0

FLUSH_BLOCK: Final = 1

FLUSH_FRAME: Final = 2

That was to address a suggestion of @emmatyping of making sure the value is only defined once.

Do you want me to revert all occurrences of _ZstdCompressorContinue?

e.g. replace _ZstdCompressorContinue | _ZstdCompressorFlushBlock | _ZstdCompressorFlushFrame down below with Literal[0, 1, 2]

The rest is fine as is. This was just to point out the redundancy here.

srittau · 2025-05-30T14:49:07Z

stdlib/_zstd.pyi

+def get_frame_info(frame_buffer: ReadableBuffer) -> tuple[int, int]: ...
+def get_frame_size(frame_buffer: ReadableBuffer) -> int: ...
+def get_param_bounds(parameter: int, is_compress: bool) -> tuple[int, int]: ...
+def set_parameter_types(c_parameter_type: type[Any], d_parameter_type: type[Any]) -> None: ...


Why use Any here? I assume that only certain types are allowed? In that case, we need a comment explaining that.

Yeah these should be specifically type[CompressionParameter] and type[DecompressionParameter].

Suggested change

def set_parameter_types(c_parameter_type: type[Any], d_parameter_type: type[Any]) -> None: ...

def set_parameter_types(c_parameter_type: type[CompressionParameter], d_parameter_type: type[DecompressionParameter]) -> None: ...

If my understanding is correct, this functions take any types, and will set valid types to be used as keys (on top of int) for the options parameters.

In the implementation of zstd a single call is done explicitly with CompressionParameter and DecompressionParameter, but it could have been something else.

In any case, it is in a private _zstd module, so I don't think it matters a lot.

Happy to change this with Emma's suggestion.

While they can take any type, I would argue it is wrong to do so. I guess you could specify type[IntEnum] perhaps if you didn't want to specify the specific types?

I think you are right, and typing with type[CompressionParameter]/type[DecompressionParameter] better shows the intent.

I will go with that.

srittau · 2025-05-30T14:53:58Z

stdlib/compression/zstd/_zstdfile.pyi

+_PathOrFileBinary: TypeAlias = StrOrBytesPath | IO[bytes]
+_PathOrFileText: TypeAlias = StrOrBytesPath | IO[str]


We try to avoid the unwieldy pseudo-protocol IO and its sub-classes, especially in argument annotations. In this case, appropriate protocols seem to be fairly to construct.

I took this from the lzma.pyi file so I thought it was ok. Tell me what you think.

The lzma stubs probably predate the introduction of protocols and need to be updated as well at some point.

srittau · 2025-05-30T14:56:30Z

stdlib/compression/zstd/_zstdfile.pyi

+    encoding: str | None = ...,
+    errors: str | None = ...,
+    newline: str | None = ...,
+) -> TextIO: ...


Same here. The implementation seems to return TextIOWrapper, so we should just return this concrete type.

I took this from the lzma.pyi file so I thought it was ok.

This one is straightforward to change though. Do you want me to change it in lzma.pyi as well?

Changing it in lzma as well would be appreciated.

srittau · 2025-05-30T14:56:36Z

stdlib/compression/zstd/_zstdfile.pyi

+    encoding: str | None = ...,
+    errors: str | None = ...,
+    newline: str | None = ...,
+) -> TextIO: ...


Rogdham · 2025-05-31T08:08:56Z

Also I notice that many defaults are ..., although they have basic defaults in the implementation.

I thought that was implementation details with no use for type checking, but I will change.

Likewise, is it needed to define the values for the constants?

For example:

# currently
ZSTD_CLEVEL_DEFAULT: Final[int]

# possible change
ZSTD_CLEVEL_DEFAULT: Final = 3

srittau · 2025-05-31T10:07:55Z

Likewise, is it needed to define the values for the constants?

Generally we do that nowadays.

github-actions · 2025-05-31T16:17:01Z

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

archinstall (https://github.com/archlinux/archinstall)
+ ./archinstall/tui/curses_menu.py:710: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 1.15.0
+ ./archinstall/tui/curses_menu.py:710: : note: use --pdb to drop into pdb
+ Traceback (most recent call last):
+   File "mypy/checkexpr.py", line 5903, in accept
+   File "mypy/nodes.py", line 1889, in accept
+   File "mypy/checkexpr.py", line 3284, in visit_member_expr
+   File "mypy/checkexpr.py", line 3309, in analyze_ordinary_member_access
+   File "mypy/checkmember.py", line 207, in analyze_member_access
+   File "mypy/checkmember.py", line 226, in _analyze_member_access
+   File "mypy/checkmember.py", line 344, in analyze_instance_member_access
+   File "mypy/meet.py", line 96, in meet_types
+   File "mypy/subtypes.py", line 223, in is_proper_subtype
+   File "mypy/subtypes.py", line 351, in _is_subtype
+   File "mypy/types.py", line 1469, in accept
+   File "mypy/subtypes.py", line 581, in visit_instance
+   File "mypy/subtypes.py", line 2095, in infer_class_variances
+   File "mypy/subtypes.py", line 2057, in infer_variance
+   File "mypy/subtypes.py", line 186, in is_subtype
+   File "mypy/subtypes.py", line 351, in _is_subtype
+   File "mypy/types.py", line 3033, in accept
+   File "mypy/subtypes.py", line 1079, in visit_partial_type
+ RuntimeError: Partial type "<partial list[?]>" cannot be checked with "issubtype()"

Rogdham · 2025-05-31T16:23:40Z

I think I have addressed all your points, feel free to review again, especially the protocols.

I am not sure what the issue with mypy_primer, feel free to provide insights if you have any!

emmatyping · 2025-05-31T18:34:43Z

I am not sure what the issue with mypy_primer, feel free to provide insights if you have any!

Definitely looks unrelated. I searched through the codebase and there aren't any references to compression.zstd

Rogdham added 2 commits May 24, 2025 13:00

Python 3.14: PEP-784 compression.zstd

219293a

Fix Self import

3ad00fc

This comment has been minimized.

Sign in to view

Rogdham added 2 commits May 24, 2025 13:35

Add to allowlist for 3.14 beta2

a37a648

Fix ZstdFile incompatibilities with parent class

ade5b67

This comment has been minimized.

Sign in to view

emmatyping reviewed May 24, 2025

View reviewed changes

stdlib/_zstd.pyi Outdated Show resolved Hide resolved

Rogdham added 2 commits May 24, 2025 19:08

Use Mapping[int, int] for options

b29dd87

Use reference for ZstdCompressor CONTINUE/FLUSH_BLOCK/FLUSH_FRAME

0f99105

This comment has been minimized.

Sign in to view

Merge branch 'origin/main'

901fecc

This comment has been minimized.

Sign in to view

Rogdham marked this pull request as ready for review May 29, 2025 07:41

srittau requested changes May 30, 2025

View reviewed changes

Explicit parameter defaults

f1e6f3d

Rogdham added 7 commits May 31, 2025 17:26

Explicit constant values

28975a0

Remove Any in set_parameter_types

c3cd99d

Change TextIO to TextIOWrapper

7177ce8

Avoid IO in file argument

b93c894

Remove redundancy in ZstdCompressor contants types

fdd5bbe

Add some more final/Final annotations

9def477

Remove Final from IntEnum members

94c7729

	def set_parameter_types(c_parameter_type: type[Any], d_parameter_type: type[Any]) -> None: ...
	def set_parameter_types(c_parameter_type: type[CompressionParameter], d_parameter_type: type[DecompressionParameter]) -> None: ...

		_PathOrFileBinary: TypeAlias = StrOrBytesPath \| IO[bytes]
		_PathOrFileText: TypeAlias = StrOrBytesPath \| IO[str]

Uh oh!

Python 3.14: PEP-784 compression.zstd #14129

Are you sure you want to change the base?

Python 3.14: PEP-784 compression.zstd #14129

Uh oh!

Conversation

Rogdham commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Rogdham commented May 24, 2025

Uh oh!

emmatyping commented May 24, 2025

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Rogdham commented May 29, 2025

Uh oh!

srittau left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rogdham commented May 31, 2025

Uh oh!

srittau commented May 31, 2025

Uh oh!

github-actions bot commented May 31, 2025

Uh oh!

Rogdham commented May 31, 2025

Uh oh!

emmatyping commented May 31, 2025

Uh oh!

Uh oh!

Rogdham commented May 24, 2025 •

edited

Loading