Repository: python/cpython
Pull request: gh-113971: Make `zipfile.ZipInfo._compresslevel` public as `.compress_level`
Make `zipfile.ZipInfo.compress_level` public.
    
A property is used to retain the behavior of the `._compresslevel`.
    
People constructing `zipfile.ZipInfo` instances to pass into existing APIs to control per-file compression levels already treat this as public, there was never a reason for it not to be.

I used the more modern name `compress_level` instead of `compresslevel` as the keyword argument on other `ZipFile` APIs is called to be consistent with `compress_type` and a general long term preference of not runningwordstogether without a separator in names.  The `zipfile` module APIs already have a variety of style inconsistencies, I don't think this makes anything worse. Someone typing the wrong one in either place will get an immediate exception.

<!-- readthedocs-preview cpython-previews start -->
----
📚 Documentation preview 📚: https://cpython-previews--113969.org.readthedocs.build/

<!-- readthedocs-preview cpython-previews end -->

<!-- gh-issue-number: gh-113971 -->
* Issue: gh-113971
<!-- /gh-issue-number -->

Fixes #113971
Closes #113971

Problem statement:
`zipfile.ZipInfo._compresslevel` should be public.
# Feature or enhancement

### Proposal:

Today people pass `ZipInfo` instances into APIs that accept them such as `ZipFile.writestr()` in order to control how individual items are put into a zip archive as such:

```python
    zip_info = zipfile.ZipInfo(filename=filename, date_time=desired_timestamp)
    zip_info.compress_type = this_files_compress_type
    # This attribute is sadly not public; manual creation of ZipInfo objects
    # is uncommon.  Without this we cannot write compressed files with our
    # own overridden info.
    zip_info._compresslevel = this_files_compress_level  # pylint: disable=protected-access
    zip_info.external_attr = desired_permissions
    zip_file.writestr(zip_info, data)
```

There is no reason for `._compresslevel` to be protected with an _. We should make it public as it is useful.

### Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

### Links to previous discussion of this feature:

_No response_

<!-- gh-linked-prs -->
### Linked PRs
* gh-113969
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: simplify the code comment.

Patch to review:
diff --git a/Doc/library/zipfile.rst b/Doc/library/zipfile.rst
index a77e49a7643826..c70f2ec561de8f 100644
--- a/Doc/library/zipfile.rst
+++ b/Doc/library/zipfile.rst
@@ -79,6 +79,11 @@ The module defines the following items:
    of the last modification to the file; the fields are described in section
    :ref:`zipinfo-objects`.
 
+   .. 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)
 
    Returns ``True`` if *filename* is a valid ZIP file based on its magic number,
diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py
index f7b6db465b4bc7..7f70976588bbd7 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 1d8a607fc728c8..8005b4b34ccf76 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,15 @@ 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
 
+    # Maintain backward compatibility with the old protected attribute name.
+    @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 +1200,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 +1612,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 +1864,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 +1884,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 +1905,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:
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 00000000000000..aa7a34d0bde71d
--- /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.


Code review comments:
diff_hunk: @@ -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):
path: Lib/test/test_zipfile/test_core.py
text: @user1:
`AbstractTestsWithSourceFile` is a wrong place for this test. `ZipInfoTests` is perhaps a correct place.

@author:
good catch, thanks!