classification
Title: Use argparse for the zipfile module
Type: enhancement Stage: patch review
Components: Demos and Tools Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: SilentGhost, berker.peksag, jaysinh.shukla, python-dev, r.david.murray, serhiy.storchaka, sjt, wolma
Priority: normal Keywords: easy, patch

Created on 2016-09-13 07:07 by serhiy.storchaka, last changed 2016-11-02 18:27 by serhiy.storchaka.

Files
File name Uploaded Description Edit
28115_2.diff SilentGhost, 2016-09-18 10:53 review
zipfile_argparse.patch serhiy.storchaka, 2016-10-16 15:38 review
zipfile-cli-required.patch serhiy.storchaka, 2016-11-02 18:27 review
Messages (13)
msg276191 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-13 07:07
Options and arguments for zipfile CLI are manually parsed for now. Since CLI becomes more complex, it is worth to convert zipfile to use argparse.
msg276303 - (view) Author: Jaysinh shukla (jaysinh.shukla) * Date: 2016-09-13 16:54
I'm working on this. :)
msg276318 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-09-13 18:23
Note that a key part of doing a translation like this is to write tests for the CLI, as comprehensively as possible, so that we have some assurance we didn't break anything in the translation.  But experience tells us that we *will* break something, since we have in every one of the translations to argparse that we've done so far :)
msg276876 - (view) Author: SilentGhost (SilentGhost) * Date: 2016-09-18 10:53
Given that at present zipfile CLI has no test or docs, and along with Guido's aversion for such tools in general, I'm not at all sure they should be added. In any case, here is the patch for the main part of conversion.
msg276891 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-18 14:20
They should be added. Otherwise we can't know that the code works as expected. You can use test_calendar.py or test_tarfile.py as a guide.
msg276893 - (view) Author: SilentGhost (SilentGhost) * Date: 2016-09-18 14:43
Neither of those fit under definition of "permissible" modules. So, I would then suggest to close this issue and any further issues against CLI of those modules.
msg277500 - (view) Author: Jaysinh shukla (jaysinh.shukla) * Date: 2016-09-27 07:33
@SilentGhost I have mad few comments on your patch. Please contact me on any confusions. Looking forward to you. Thanks!
msg277518 - (view) Author: SilentGhost (SilentGhost) * Date: 2016-09-27 15:38
Jaysinh, you can upload your modified patch, so it can be reviewed.
msg278769 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-16 15:38
Proposed patch makes zipfile using argparse and adds tests for zipfile CLI. Originally it is based on tarfile code and tests (but reworked, especially tests). Basically the interface is not changed, but added support of long options, and exit code in case of calling without options is changed from 1 to 2 (the same as wrong option is specified). The format of help and usage info is changed of course.

Tests are passed with old releases of Python (except supporting long options). They will be backported.
msg279249 - (view) Author: Roundup Robot (python-dev) Date: 2016-10-23 10:18
New changeset 042c923c5b67 by Serhiy Storchaka in branch '2.7':
Issue #28115: Added tests for CLI of the zipfile module.
https://hg.python.org/cpython/rev/042c923c5b67

New changeset 900c47c98711 by Serhiy Storchaka in branch '3.5':
Issue #28115: Added tests for CLI of the zipfile module.
https://hg.python.org/cpython/rev/900c47c98711

New changeset a1975621bba2 by Serhiy Storchaka in branch '3.6':
Issue #28115: Added tests for CLI of the zipfile module.
https://hg.python.org/cpython/rev/a1975621bba2

New changeset 5edac3b55130 by Serhiy Storchaka in branch 'default':
Issue #28115: Added tests for CLI of the zipfile module.
https://hg.python.org/cpython/rev/5edac3b55130
msg279251 - (view) Author: Roundup Robot (python-dev) Date: 2016-10-23 12:09
New changeset fa275e570d52 by Serhiy Storchaka in branch 'default':
Issue #28115: Command-line interface of the zipfile module now uses argparse.
https://hg.python.org/cpython/rev/fa275e570d52
msg279276 - (view) Author: Roundup Robot (python-dev) Date: 2016-10-23 19:34
New changeset 7f01d9d471e5 by Serhiy Storchaka in branch '2.7':
Issue #28115: ZIP creation test requires zlib.
https://hg.python.org/cpython/rev/7f01d9d471e5

New changeset 7701e9cb8712 by Serhiy Storchaka in branch '3.5':
Issue #28115: ZIP creation test requires zlib.
https://hg.python.org/cpython/rev/7701e9cb8712

New changeset 5b779441d03e by Serhiy Storchaka in branch '3.6':
Issue #28115: ZIP creation test requires zlib.
https://hg.python.org/cpython/rev/5b779441d03e

New changeset 3e7da46aead3 by Serhiy Storchaka in branch 'default':
Issue #28115: ZIP creation test requires zlib.
https://hg.python.org/cpython/rev/3e7da46aead3
msg279937 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-02 18:27
SilentGhost suggested to pass required=True to add_mutually_exclusive_group(). This makes the last "else" not needed.

Proposed patch implements this suggestion.
History
Date User Action Args
2016-11-02 18:27:34serhiy.storchakasetfiles: + zipfile-cli-required.patch
assignee: serhiy.storchaka
messages: + msg279937
2016-10-23 19:34:06python-devsetmessages: + msg279276
2016-10-23 12:09:21python-devsetmessages: + msg279251
2016-10-23 10:18:13python-devsetnosy: + python-dev
messages: + msg279249
2016-10-18 07:51:09wolmasetnosy: + wolma
2016-10-16 15:38:49serhiy.storchakasetfiles: + zipfile_argparse.patch

messages: + msg278769
2016-09-27 15:38:04SilentGhostsetmessages: + msg277518
2016-09-27 07:45:47serhiy.storchakasetnosy: + berker.peksag
2016-09-27 07:33:43jaysinh.shuklasetmessages: + msg277500
2016-09-18 14:43:59SilentGhostsetmessages: + msg276893
2016-09-18 14:20:01serhiy.storchakasetmessages: + msg276891
2016-09-18 10:53:58SilentGhostsetfiles: + 28115_2.diff
keywords: + patch
messages: + msg276876

stage: needs patch -> patch review
2016-09-13 18:23:35r.david.murraysetnosy: + r.david.murray
messages: + msg276318
2016-09-13 16:54:30jaysinh.shuklasetnosy: + jaysinh.shukla
messages: + msg276303
2016-09-13 07:07:03serhiy.storchakacreate