Repository navigation
zipfile.Path.read_text & .open methods with a positional encoding arg causes a TypeError #101144
Description
This is a regression from 3.9 behavior seen in Python 3.10.
zipfile.Path.read_text passes *args and **kwargs to it's own open() method, but in 3.10+ is also blindly sets kwargs["encodings"] to a value. So code that was previously passing an encoding as a positional argument now sees:
File "/<stdlib>/zipfile.py", line 2362, in read_text
with self.open('r', *args, **kwargs) as strm:
File "/<stdlib>/zipfile.py", line 2350, in open
return io.TextIOWrapper(stream, *args, **kwargs)
TypeError: Failed to construct dataset imagenet2012: argument for TextIOWrapper() given by name ('encoding') and position (2)
3.10's Lib/zipfile.py (and main's Lib/zipfile/_path.py) contain:
def read_text(self, *args, **kwargs):
kwargs["encoding"] = io.text_encoding(kwargs.get("encoding")) # <-- new in 3.10, source of bug
with self.open('r', *args, **kwargs) as strm:
return strm.read()
As this is a regression, we should avoid setting that value in kwargs when "encodings" not in kwargs to match 3.9 and earlier behavior.
TODO list:
Activity
gpshead commentedon Jan 19, 2023
(and note that 3.10 is probably closed to bugfixes)
gpshead commentedon Jan 19, 2023
Workaround for users, always pass encoding to read_text as a keyword encoding= argument rather than the first positional argument.
[-]`zipfile.Path.read_text` with a positional encoding arg causes argument for TextIOWrapper() given by name ('encoding') and position (2)[/-][+]`zipfile.Path` read_text and open methods with positional encoding arg causes TypeError argument given by name ('encoding') and position (2)[/+]pythongh-101144: Allow open and read_text encoding to be positional.
[-]`zipfile.Path` read_text and open methods with positional encoding arg causes TypeError argument given by name ('encoding') and position (2)[/-][+]`zipfile.Path.read_text` & `.open` methods with a positional `encoding` arg causes a TypeError[/+]jaraco commentedon Jan 19, 2023
One thought that I keep having - although this incompatibility was introduced unintentionally, it seems not to have affected but one reported use-case (and probably a handful that were not reported). Perhaps it makes sense to retain the more constrained form. As I consider that, I realize that these wrappers are really trying to be true to the underlying interface, so it's probably preferable to honor that and remain consistent than to offer only a more constrained interface. In other words, the proposed fix sounds good.
jaraco commentedon Jan 19, 2023
Feel free to assign to me after the PR lands and I can handle the port. Or if you're interested in learning the process, I can share the process with you.
gh-101144: Allow open and read_text encoding to be positional. (#101145)
[3.11] pythongh-101144: Allow open and read_text encoding to be posit…
[3.11] gh-101144: Allow zipfile.Path .open & .read_text encoding to b…
[3.11] pythongh-101144: Allow zipfile.Path .open & .read_text encodin…
gpshead commentedon Jan 20, 2023
yep, I went through the same thought process. They're mirroring other APIs that take encoding and those accept it as a positional parameter, so fixing it rather than just declaring it keyword only made sense as a bugfix.
As for your
zippproject @jaraco, I'll let you apply the change there. Keeping this open and reassigning to you for that. The 3.10 PR will probably automerge, if not one of us can push the button when CI is done.[3.10] gh-101144: Allow zipfile.Path .open & .read_text encoding to b…
python/cpython#101144: Allow open and read_text encoding to be positi…
Update changelog. Ref python/cpython#101144.
python/cpython#101144: Allow open and read_text encoding to be positi…
Update changelog. Ref python/cpython#101144.
Update changelog. Ref python/cpython#101144.
jaraco commentedon Jan 27, 2023
The backport ended up being fairly challenging, given the lack of
sys.flags.warn_default_encodingandEncodingWarningon Python prior to 3.10, but also the nitpickiness of mypy and flake8. I could satisfy flake8 by monkeypatching builtins and sys.flags, but I couldn't satisfy mypy because it has the attributes ofsys.flagshard-coded somewhere. Ultimately, I came up with something that's suitable and pretty true to the same spirit in jaraco/zipp#89 released as zipp 3.12 (the relationship to Python 3.12 is purely coincidental). The one downside is that the check fortest_encoding_warningsis now skipped except in environments where the value is set. I'm assuming that the test suite is regularly tested with-X warn_default_encoding, so the test will catch a regression soon enough.I plan to add
PYTHONWARNDEFAULTENCODINGto skeleton so that the projects I maintain get the warnings.pypi-zipp: Autospec creation for update from version 3.11.0 to versio…