Shouldn't the ZipFile.filename attribute be converted to str?

If add support of pathlib, maybe add support of bytes?

The file name of ZipFile is only a part of the issue. There are other uses of file paths: paths for added files, path for extracted directory.

And there are internal paths in zip archive. Maybe it is worth to introduce new path-like type for them and provide pathlib-like interface to ZipFile. But this is separate not easy issue.

the patch LGTM, but Serhiy has a point that maybe we should add tests for dealing with other paths such as those contained within the zipfile.

This also affects python 3.5 on Windows and OSX.

Python 3.5.2 (default, Sep 21 2016, 15:07:18)
[GCC 4.2.1 Compatible Apple LLVM 7.3.0 (clang-703.0.31)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pathlib import Path
>>> import zipfile
>>> f = Path('zipfile.zip')
>>> with zipfile.ZipFile(f) as zf:
... 	pass
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jeremy/.pyenv/versions/3.5.2/lib/python3.5/zipfile.py", line 1026, in __init__
    self._RealGetContents()
  File "/Users/jeremy/.pyenv/versions/3.5.2/lib/python3.5/zipfile.py", line 1089, in _RealGetContents
    endrec = _EndRecData(fp)
  File "/Users/jeremy/.pyenv/versions/3.5.2/lib/python3.5/zipfile.py", line 241, in _EndRecData
    fpin.seek(0, 2)
AttributeError: 'PosixPath' object has no attribute 'seek'

In 3.5 the stdlib is not supporting PathLib. So this issue only affects 3.6 and 3.7.

OK, I understand. How can I help get this issue fixed?

1. review the patch?
2. update the docs to reflect the patch?
3. find the other uses of pathlib in the zipfile module?
4. something else ...

I am a longtime user of python but a first time contributor but happy to help.

I think the next steps are:

1. Add tests for the cases Serhiy has mentioned in msg277109:

   > Shouldn't the ZipFile.filename attribute be converted to str?

   and

   > The file name of ZipFile is only a part of the issue. There are other uses of file paths: paths for added files, path for extracted directory.

2. Update the docs to reflect the pathlib support.

I have reviewed the code and docs for the public API that should take a pathlib.Path object:

• zipfile.is_zipfile(filename)
  • filename
• zipfile.ZipFile(file)
  • file
• ZipFile.extract(member, path=None)
  • path
• ZipFile.extractall(path=None)
  • path
• ZipFile.write(filename)
  • filename
• zipfile.PyZipFile(file)
  • file
• PyZipFile.writepy(pathname)
  • pathname
• ZipInfo.from_file(filename, arcname=None)
  • filename

Does this appear complete?

Working on tests that probe each of these API points with pathlib.Path objects.

I am not sure what "Shouldn't the ZipFile.filename attribute be converted to str?" means, can you elaborate?

I am not sure what "Shouldn't the ZipFile.filename attribute be converted to str?" means, can you elaborate?

If you pass a Path object to ZipFile constructor, should the filename attribute be a Path object or a str?

Any path/file attributes, etc, inside a ZipFile should be str. ZipFile should also function properly if path/file requests are given as os.PathLike objects.

Speaking as a "regular user" who just ran into this, my main concern is that PathLike paths get used properly. For filenames being passed back out, if I really want them to be Path objects, I'll wrap them in Path() anyway.

Please don't let a full conversion to pathlib hold up fixing passing a PathLike into the constructor. That's the main use case, and given how nicely most of the rest of the stdlib handles Path objects now, it's an annoying wart.

PR 322 should make the example in msg284754 work:

>>> import pathlib, zipfile
>>> f = pathlib.Path('spam.zip')
>>> with zipfile.ZipFile(f) as zf:
...   zf.namelist()
... 
['LICENSE']

It doesn't implement full PathLike support, but it at least covers the use cases of Jeremy and Steve.

I have different path. It adds the support of path-like objects for all external paths.

Why can't we fix this in 3.6? We were meant to support pathlike in that version, and this is an oversight, not a new feature.

I consider this a new feature.

Some modules got the support of path-like objects for free after implementing the support of path-like objects in os, os.path and io modules. But others need additional work for implementing it, writing tests and documentation. In case of zipfile this work is significant.

Note that Ned gave us a permission to get this into 3.6.1.

Note that Ned gave us a permission to get this into 3.6.1.

I may have although I don't remember specifically discussing zipfile. In any case, I'm willing to consider it. I think you can make good arguments for and against. Yes, it could smell like adding a feature but, on the other add, one of the implicit goals of 3.6.0 was to make Path objects supported across the standard library as much as possible, so the lack of support in zipfile (and a few other similar modules) could be viewed as bug. Also, as far as I can tell, this should be a totally upwards-compatible change except in the presumably unlikely case something is counting on getting an exception when passing a Path object to zipfile. I say we invoke "practicality beats purity" for this as long as Serhiy is OK with having it cherry-picked to 3.6 and as long as no other core developer here has a strong objection.

The patch is backported to 3.6.1.

New changeset eb65edd by Serhiy Storchaka in branch '3.6':
[3.6] bpo-28231: The zipfile module now accepts path-like objects for external paths. (#561)
eb65edd

New changeset 8606e95 by Serhiy Storchaka in branch 'master':
bpo-28231: The zipfile module now accepts path-like objects for external paths. (#511)
8606e95