bpo-32742: use infolist for zipfile.ZipFile.extractall #5472
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
zipfile.ZipFile class's extractall method allows the list of members to be unset and if it is, defaults to the list of file *names* in the zip. When sent to _extract_member that file name needs to be rewrapped as a ZipInfo instance. The ZipInfo instances is already available inside extractall and can be re-used.
peterbe
force-pushed
the
use-infolist-for-zipefileextractall
branch
from
d802d06 to
1bb1df5
Compare
Lib/zipfile.py
Outdated
| directory. `path' specifies a different directory to extract to. | ||
| `members' is optional and must be a subset of the list returned | ||
| by namelist(). | ||
| by infolist(). |
There was a problem hiding this comment.
I think more doc is needed (here and in the ReST documentation) to make things clear. The code change in this PR does not change the function interface: people could pass a list of names or a list of ZipInfos before (thanks to extractall calling _extract_member which supports both), and they’re still able to do that after the change.
Maybe the doc could be explicit about these two options? Instead of naming directly namelist or infolist (and letting people think that members must be strings or must be ZipInfos), something like «members is optional and must contain strings or ZipInfo objects corresponding to valid entries (i.e. a subset of the list returned by namelist or infolist)».
There was a problem hiding this comment.
Thanks. I juggled the idea but everything I attempted just got long winded and more confusing. Just saying infolist or namelist is both correct and really brief. Too brief?
|
Status: There is a question on the bug report about a behaviour change that needs to be addressed. |
|
This PR is stale because it has been open for 30 days with no activity. |
5 participants
https://bugs.python.org/issue32742