Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use argparse for the zipfile module #72302

Closed
serhiy-storchaka opened this issue Sep 13, 2016 · 14 comments
Closed

Use argparse for the zipfile module #72302

serhiy-storchaka opened this issue Sep 13, 2016 · 14 comments
Assignees
@serhiy-storchaka
Labels
3.7 (EOL) end of life easy type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 28115
Nosy @bitdancer, @yaseppochi, @berkerpeksag, @serhiy-storchaka, @wm75, @ultimatecoder
Files
  • 28115_2.diff
  • zipfile_argparse.patch
  • zipfile-cli-required.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-04-01.06:41:51.525>
created_at = <Date 2016-09-13.07:07:03.682>
labels = ['easy', 'type-feature', '3.7']
title = 'Use argparse for the zipfile module'
updated_at = <Date 2017-04-01.06:41:51.524>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-04-01.06:41:51.524>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-04-01.06:41:51.525>
closer = 'serhiy.storchaka'
components = ['Demos and Tools']
creation = <Date 2016-09-13.07:07:03.682>
creator = 'serhiy.storchaka'
dependencies = []
files = ['44735', '45117', '45316']
hgrepos = []
issue_num = 28115
keywords = ['patch', 'easy']
message_count = 14.0
messages = ['276191', '276303', '276318', '276876', '276891', '276893', '277500', '277518', '278769', '279249', '279251', '279276', '279937', '290967']
nosy_count = 8.0
nosy_names = ['r.david.murray', 'sjt', 'SilentGhost', 'python-dev', 'berker.peksag', 'serhiy.storchaka', 'wolma', 'jaysinh.shukla']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28115'
versions = ['Python 3.7']
    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life easy type-feature A feature request or enhancement labels Sep 13, 2016
    @ultimatecoder
    Copy link
    Mannequin

    ultimatecoder mannequin commented Sep 13, 2016

    I'm working on this. :)

    @bitdancer
    Copy link
    Member

    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 :)

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Sep 18, 2016

    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.

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Sep 18, 2016

    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.

    @ultimatecoder
    Copy link
    Mannequin

    ultimatecoder mannequin commented Sep 27, 2016

    @SilentGhost I have mad few comments on your patch. Please contact me on any confusions. Looking forward to you. Thanks!

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Sep 27, 2016

    Jaysinh, you can upload your modified patch, so it can be reviewed.

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 23, 2016

    New changeset 042c923c5b67 by Serhiy Storchaka in branch '2.7':
    Issue bpo-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 bpo-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 bpo-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 bpo-28115: Added tests for CLI of the zipfile module.
    https://hg.python.org/cpython/rev/5edac3b55130

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 23, 2016

    New changeset fa275e570d52 by Serhiy Storchaka in branch 'default':
    Issue bpo-28115: Command-line interface of the zipfile module now uses argparse.
    https://hg.python.org/cpython/rev/fa275e570d52

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 23, 2016

    New changeset 7f01d9d471e5 by Serhiy Storchaka in branch '2.7':
    Issue bpo-28115: ZIP creation test requires zlib.
    https://hg.python.org/cpython/rev/7f01d9d471e5

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

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

    New changeset 3e7da46aead3 by Serhiy Storchaka in branch 'default':
    Issue bpo-28115: ZIP creation test requires zlib.
    https://hg.python.org/cpython/rev/3e7da46aead3

    @serhiy-storchaka
    Copy link
    Member Author

    SilentGhost suggested to pass required=True to add_mutually_exclusive_group(). This makes the last "else" not needed.

    Proposed patch implements this suggestion.

    @serhiy-storchaka serhiy-storchaka self-assigned this Nov 2, 2016
    @serhiy-storchaka
    Copy link
    Member Author

    Opened bpo-29958 for the latter patch.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.7 (EOL) end of life easy type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @bitdancer @serhiy-storchaka