From 2b7fa231e14a14f52d6665e5dc03bc7971f3c059 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 11 Jan 2024 15:15:48 -0800 Subject: [PATCH 1/6] Make `zipfile.ZipInfo.compress_level` public. A property is used to retain the behavior of the `._compresslevel`. People constructing ZipInfo instances to pass in to control per-file compression levels already treat this as public, there was never a reason for it not to be. --- Lib/test/test_zipfile/test_core.py | 15 +++++++++++++-- Lib/zipfile/__init__.py | 30 +++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index f7b6db465b4bc7c..7f70976588bbd72 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -315,7 +315,7 @@ def test_writestr_compresslevel(self): # Compression level follows the constructor. a_info = zipfp.getinfo('a.txt') self.assertEqual(a_info.compress_type, self.compression) - self.assertEqual(a_info._compresslevel, 1) + self.assertEqual(a_info.compress_level, 1) # Compression level is overridden. b_info = zipfp.getinfo('b.txt') @@ -408,7 +408,7 @@ def test_per_file_compresslevel(self): one_info = zipfp.getinfo('compress_1') nine_info = zipfp.getinfo('compress_9') self.assertEqual(one_info._compresslevel, 1) - self.assertEqual(nine_info._compresslevel, 9) + self.assertEqual(nine_info.compress_level, 9) def test_writing_errors(self): class BrokenFile(io.BytesIO): @@ -645,6 +645,17 @@ def test_add_file_after_2107(self): zinfo = zipfp.getinfo(TESTFN) self.assertEqual(zinfo.date_time, (2107, 12, 31, 23, 59, 59)) + def test_ZipInfo_compresslevel_property(self): + zinfo = zipfile.ZipInfo("xxx") + self.assertFalse(zinfo._compresslevel) + self.assertFalse(zinfo.compress_level) + zinfo._compresslevel = 99 # test the legacy @property.setter + self.assertEqual(zinfo.compress_level, 99) + self.assertEqual(zinfo._compresslevel, 99) + zinfo.compress_level = 8 + self.assertEqual(zinfo.compress_level, 8) + self.assertEqual(zinfo._compresslevel, 8) + @requires_zlib() class DeflateTestsWithSourceFile(AbstractTestsWithSourceFile, diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 1d8a607fc728c89..1ae58e04916cfb3 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -371,7 +371,7 @@ def _sanitize_filename(filename): return filename -class ZipInfo (object): +class ZipInfo: """Class with attributes describing each file in the ZIP archive.""" __slots__ = ( @@ -379,7 +379,7 @@ class ZipInfo (object): 'filename', 'date_time', 'compress_type', - '_compresslevel', + 'compress_level', 'comment', 'extra', 'create_system', @@ -413,7 +413,7 @@ def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)): # Standard values: self.compress_type = ZIP_STORED # Type of compression for the file - self._compresslevel = None # Level for the compressor + self.compress_level = None # Level for the compressor self.comment = b"" # Comment for each file self.extra = b"" # ZIP extra data if sys.platform == 'win32': @@ -435,6 +435,18 @@ def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)): # header_offset Byte offset to the file header # CRC CRC-32 of the uncompressed file + # Historically ._compresslevel was private for no reason. People used it as + # a public attribute when constructing their own ZipInfo to control the + # setting on a per file basis when added to an archive. We moved it into + # .compress_level, this property retains compatibility. + @property + def _compresslevel(self): + return self.compress_level + + @_compresslevel.setter + def _compresslevel(self, value): + self.compress_level = value + def __repr__(self): result = ['<%s filename=%r' % (self.__class__.__name__, self.filename)] if self.compress_type != ZIP_STORED: @@ -1191,7 +1203,7 @@ def __init__(self, zf, zinfo, zip64): self._zip64 = zip64 self._zipfile = zf self._compressor = _get_compressor(zinfo.compress_type, - zinfo._compresslevel) + zinfo.compress_level) self._file_size = 0 self._compress_size = 0 self._crc = 0 @@ -1603,7 +1615,7 @@ def open(self, name, mode="r", pwd=None, *, force_zip64=False): elif mode == 'w': zinfo = ZipInfo(name) zinfo.compress_type = self.compression - zinfo._compresslevel = self.compresslevel + zinfo.compress_level = self.compresslevel else: # Get info object for name zinfo = self.getinfo(name) @@ -1855,9 +1867,9 @@ def write(self, filename, arcname=None, zinfo.compress_type = self.compression if compresslevel is not None: - zinfo._compresslevel = compresslevel + zinfo.compress_level = compresslevel else: - zinfo._compresslevel = self.compresslevel + zinfo.compress_level = self.compresslevel with open(filename, "rb") as src, self.open(zinfo, 'w') as dest: shutil.copyfileobj(src, dest, 1024*8) @@ -1875,7 +1887,7 @@ def writestr(self, zinfo_or_arcname, data, zinfo = ZipInfo(filename=zinfo_or_arcname, date_time=time.localtime(time.time())[:6]) zinfo.compress_type = self.compression - zinfo._compresslevel = self.compresslevel + zinfo.compress_level = self.compresslevel if zinfo.filename.endswith('/'): zinfo.external_attr = 0o40775 << 16 # drwxrwxr-x zinfo.external_attr |= 0x10 # MS-DOS directory flag @@ -1896,7 +1908,7 @@ def writestr(self, zinfo_or_arcname, data, zinfo.compress_type = compress_type if compresslevel is not None: - zinfo._compresslevel = compresslevel + zinfo.compress_level = compresslevel zinfo.file_size = len(data) # Uncompressed size with self._lock: From c2eb2585d82ebaed1336c5fde7e0503865342c06 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 11 Jan 2024 15:23:13 -0800 Subject: [PATCH 2/6] Documentation. --- Doc/library/zipfile.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index a77e49a76438267..e3c821c4854289f 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -79,6 +79,12 @@ The module defines the following items: of the last modification to the file; the fields are described in section :ref:`zipinfo-objects`. + .. versionchanged:: 3.13 + While the internals of a :class:`!ZipInfo` objects remain + un(der)documented, the ``.compress_level`` attribute was renamed from + ``._compresslevel``. The old name continues to work as a property for + backwards compatibility. + .. function:: is_zipfile(filename) Returns ``True`` if *filename* is a valid ZIP file based on its magic number, From 6043a1ada7b466ba937403461411430db8e531fb Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 11 Jan 2024 16:58:13 -0800 Subject: [PATCH 3/6] NEWS entry. --- .../Library/2024-01-11-16-58-10.gh-issue-113971.skJZ4g.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-01-11-16-58-10.gh-issue-113971.skJZ4g.rst diff --git a/Misc/NEWS.d/next/Library/2024-01-11-16-58-10.gh-issue-113971.skJZ4g.rst b/Misc/NEWS.d/next/Library/2024-01-11-16-58-10.gh-issue-113971.skJZ4g.rst new file mode 100644 index 000000000000000..aa7a34d0bde71dc --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-11-16-58-10.gh-issue-113971.skJZ4g.rst @@ -0,0 +1,4 @@ +The :class:`zipfile.ZipInfo` previously protected ``._compresslevel`` +attribute has been made public as ``.compress_level`` with the old +``_compresslevel`` name remaining available as a property to retain +compatibility. From f2a43025bf9e18d038d00630c0e07cb3ae5eae79 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 11 Jan 2024 17:01:39 -0800 Subject: [PATCH 4/6] Reword the doc as versionadded. --- Doc/library/zipfile.rst | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst index e3c821c4854289f..c70f2ec561de8f5 100644 --- a/Doc/library/zipfile.rst +++ b/Doc/library/zipfile.rst @@ -79,11 +79,10 @@ The module defines the following items: of the last modification to the file; the fields are described in section :ref:`zipinfo-objects`. - .. versionchanged:: 3.13 - While the internals of a :class:`!ZipInfo` objects remain - un(der)documented, the ``.compress_level`` attribute was renamed from - ``._compresslevel``. The old name continues to work as a property for - backwards compatibility. + .. versionadded:: 3.13 + A public ``.compress_level`` attribute has been added to expose the + formerly protected ``._compresslevel``. The older protected name + continues to work as a property for backwards compatibility. .. function:: is_zipfile(filename) From 8efefff3fa72fe84f92581c3b66b2e4d8edcdbb3 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Thu, 11 Jan 2024 17:04:10 -0800 Subject: [PATCH 5/6] simplify the code comment. --- Lib/zipfile/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 1ae58e04916cfb3..8005b4b34ccf76c 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -435,10 +435,7 @@ def __init__(self, filename="NoName", date_time=(1980,1,1,0,0,0)): # header_offset Byte offset to the file header # CRC CRC-32 of the uncompressed file - # Historically ._compresslevel was private for no reason. People used it as - # a public attribute when constructing their own ZipInfo to control the - # setting on a per file basis when added to an archive. We moved it into - # .compress_level, this property retains compatibility. + # Maintain backward compatibility with the old protected attribute name. @property def _compresslevel(self): return self.compress_level From 62e06b8331cee01d3528865219af559a7ef77eed Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Fri, 12 Jan 2024 11:56:02 -0800 Subject: [PATCH 6/6] Move the test into ZipInfoTests. --- Lib/test/test_zipfile/test_core.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 7f70976588bbd72..9bdb08aeabb7817 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -645,17 +645,6 @@ def test_add_file_after_2107(self): zinfo = zipfp.getinfo(TESTFN) self.assertEqual(zinfo.date_time, (2107, 12, 31, 23, 59, 59)) - def test_ZipInfo_compresslevel_property(self): - zinfo = zipfile.ZipInfo("xxx") - self.assertFalse(zinfo._compresslevel) - self.assertFalse(zinfo.compress_level) - zinfo._compresslevel = 99 # test the legacy @property.setter - self.assertEqual(zinfo.compress_level, 99) - self.assertEqual(zinfo._compresslevel, 99) - zinfo.compress_level = 8 - self.assertEqual(zinfo.compress_level, 8) - self.assertEqual(zinfo._compresslevel, 8) - @requires_zlib() class DeflateTestsWithSourceFile(AbstractTestsWithSourceFile, @@ -3022,6 +3011,17 @@ def test_from_dir(self): self.assertEqual(zi.compress_type, zipfile.ZIP_STORED) self.assertEqual(zi.file_size, 0) + def test_compresslevel_property(self): + zinfo = zipfile.ZipInfo("xxx") + self.assertFalse(zinfo._compresslevel) + self.assertFalse(zinfo.compress_level) + zinfo._compresslevel = 99 # test the legacy @property.setter + self.assertEqual(zinfo.compress_level, 99) + self.assertEqual(zinfo._compresslevel, 99) + zinfo.compress_level = 8 + self.assertEqual(zinfo.compress_level, 8) + self.assertEqual(zinfo._compresslevel, 8) + class CommandLineTest(unittest.TestCase):