- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
          gh-81719: Add private members to zipfile.ZipFile to make it easier to subclass
          #137101
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
54b7e31
              609d44f
              a6d22b7
              36defbc
              a3be112
              2c998fb
              63856e8
              d7baea1
              83e7979
              7ac534a
              7be0dab
              b5edb6f
              54e6384
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|  | @@ -5,6 +5,7 @@ | |||||
| import io | ||||||
| import itertools | ||||||
| import os | ||||||
| import pathlib | ||||||
| import posixpath | ||||||
| import stat | ||||||
| import struct | ||||||
|  | @@ -485,6 +486,9 @@ def tearDown(self): | |||||
|  | ||||||
| class StoredTestsWithSourceFile(AbstractTestsWithSourceFile, | ||||||
| unittest.TestCase): | ||||||
| """ | ||||||
| Test in which the files inside the archive are not compressed. | ||||||
| """ | ||||||
|          | ||||||
| compression = zipfile.ZIP_STORED | ||||||
| test_low_compression = None | ||||||
|  | ||||||
|  | @@ -676,6 +680,104 @@ def test_add_file_after_2107(self): | |||||
| self.assertEqual(zinfo.date_time, (2107, 12, 31, 23, 59, 59)) | ||||||
|  | ||||||
|  | ||||||
| class CustomZipInfo(zipfile.ZipInfo): | ||||||
| """ | ||||||
|          | ||||||
| Support for testing extending and subclassing ZipFile. | ||||||
| """ | ||||||
|  | ||||||
| class CustomZipExtFile(zipfile.ZipExtFile): | ||||||
| """ | ||||||
| Support for testing extending and subclassing ZipFile. | ||||||
| """ | ||||||
|  | ||||||
| def test_read_custom_zipinfo_and_zipextfile(self): | ||||||
| """ | ||||||
| A subclass of ZipFile can be implemented to read and handle the | ||||||
| archive content using custom ZipInfo and ZipExtFile implementations. | ||||||
| """ | ||||||
| # Create the file using the default Zipfile. | ||||||
| source = io.BytesIO() | ||||||
| with zipfile.ZipFile(source, 'w', zipfile.ZIP_STORED) as zipfp: | ||||||
| zipfp.writestr('test.txt', 'some-text-content') | ||||||
|  | ||||||
| with zipfile.ZipFile( | ||||||
| source, 'r', | ||||||
| zipinfo_class=self.CustomZipInfo, | ||||||
| zipextfile_class=self.CustomZipExtFile, | ||||||
| ) as zipfp: | ||||||
| # Archive content returns the custom ZipInfo | ||||||
| members = zipfp.infolist() | ||||||
| self.assertEqual(1, len(members)) | ||||||
| self.assertIsInstance(members[0], self.CustomZipInfo) | ||||||
|  | ||||||
| # Archive members can be opened using the custom ZipInfo | ||||||
| target_member = members[0] | ||||||
| with zipfp.open(target_member, mode='r') as memberfp: | ||||||
| self.assertIsInstance(memberfp, self.CustomZipExtFile) | ||||||
|  | ||||||
| def test_write_custom_zipinfo(self): | ||||||
| """ | ||||||
| A subclass of ZipFile can be implemented to write and handle the | ||||||
| archive content using custom ZipInfo implementation. | ||||||
| """ | ||||||
| destination = io.BytesIO() | ||||||
| with zipfile.ZipFile( | ||||||
| destination, 'w', zipinfo_class=self.CustomZipInfo) as zipfp: | ||||||
| # It can write using the specific custom classe. | ||||||
|          | ||||||
| # It can write using the specific custom classe. | |
| # It can write using the specific custom class. | 
        
          
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid using private members in tests. If we change how they work, it adds additional maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Make sense.
        
          
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to just use Path(source_dir) / 'source.txt' here.
        
          
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this part of the test is necessary. We aren't stressing anything on the custom classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to test this change
    def _extract_member(self, member, targetpath, pwd):
        """Extract the ZipInfo object 'member' to a physical
           file on the path targetpath.
        """
-        if not isinstance(member, ZipInfo):
+        if not isinstance(member, self._ZipInfo):
            member = self.getinfo(member)There is not much public API to check for this method.
I have updated the assertions as an end to end test.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| :class:`zipfile.ZipFile` was given the ``zipinfo_class`` and ``zipextfile_class`` to make it easier to subclass and | ||
| extend it. | ||
|         
                  brianschubert marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
Uh oh!
There was an error while loading. Please reload this page.