GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-101979: fix a bug that parentheses in metavar argument of add_argument() were dropped by yeojin-dev · Pull Request #102318 · python/cpython

Conversation

yeojin-dev commented Feb 28, 2023 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Remove a regular expression that removes parentheses. Instead, add parentheses only if you use two or more non required and non suppressed actions.

Issue: argparse metavar parentheses dropped on usage line #101979

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

uyw4687 reacted with thumbs up emoji

All reactions

👍 1 reaction

yeojin-dev and others added 6 commits

February 28, 2023 16:04


          Delete a redundant cleanup

4d68880


          Add parenthesis only when there are more than two required actions

70ef4fc


          Count only exposed actions

da07d8c


          Add testcase

4c6889c


          Add NEWS.d

0dc91c0


          Fix typo

7f82863

bedevere-bot added the awaiting review label

Feb 28, 2023

bedevere-bot mentioned this pull request

Feb 28, 2023

argparse metavar parentheses dropped on usage line #101979

Closed

corona10 self-assigned this

Feb 28, 2023

corona10 self-requested a review

February 28, 2023 11:12

corona10 assigned JelleZijlstra and unassigned corona10

Mar 1, 2023

corona10 commented Mar 1, 2023

Copy link

Copy Markdown

Member

@JelleZijlstra
Can you please take a look? I think that I am not argparse expert to review this PR.
@yeojin-dev is one of our internal sprint members :)

yeojin-dev reacted with heart emoji

All reactions

❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

corona10 requested a review from JelleZijlstra

March 1, 2023 06:19

corona10 reviewed

Mar 1, 2023

View reviewed changes

Comment thread

Lib/argparse.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

corona10 reviewed

Mar 1, 2023

View reviewed changes

Comment thread

Lib/argparse.py Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Refactor code

74c14ce

JelleZijlstra reviewed

Mar 3, 2023

View reviewed changes

Comment thread

Misc/NEWS.d/next/Library/2023-02-28-09-52-25.gh-issue-101979.or3hXV.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra reviewed

Mar 3, 2023

View reviewed changes

JelleZijlstra left a comment

Copy link

Copy Markdown

Member

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.

Looks good apart from minor

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_argparse.py

		version = ''


		class TestHelpUsageWithParentheses(HelpTestCase):

JelleZijlstra Mar 3, 2023

Copy link

Copy Markdown

Member

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 don't see any tests that actually add parentheses for a group with more than one action. Could you add one?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

yeojin-dev reacted with thumbs up emoji

All reactions

👍 1 reaction

yeojin-dev Mar 5, 2023

Copy link

Copy Markdown

Contributor Author

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.

That's a good idea. But I think the testcase you want already exists using the code below. Please review it.

cpython/Lib/test/test_argparse.py

Line 2777 in 66aa78c

usage: PROG [-h] (--bar BAR | --baz [BAZ])

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JelleZijlstra Mar 5, 2023

Copy link

Copy Markdown

Member

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're right, thanks! There are a lot of tests, I must have glanced over that one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

yeojin-dev reacted with heart emoji

All reactions

❤️ 1 reaction


          Update Misc/NEWS.d/next/Library/2023-02-28-09-52-25.gh-issue-101979.o…

b05fb16

…r3hXV.rst

Co-authored-by: Jelle Zijlstra <[email protected]>

yeojin-dev commented Mar 5, 2023

Copy link

Copy Markdown

Contributor Author

I have made the requested changes; please review again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting change review and removed awaiting review labels

Mar 5, 2023

bedevere-bot commented Mar 5, 2023

Copy link

Copy Markdown

Thanks for making the requested changes!

: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra approved these changes

Mar 5, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels

Mar 5, 2023

JelleZijlstra added needs backport to 3.10 needs backport to 3.11 only security fixes labels

Mar 5, 2023

JelleZijlstra merged commit 9a478be into python:main

Mar 5, 2023

miss-islington commented Mar 5, 2023

Copy link

Copy Markdown

Contributor

Thanks @yeojin-dev for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the awaiting merge label

Mar 5, 2023

bedevere-bot commented Mar 5, 2023

Copy link

Copy Markdown

GH-102438 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.11 only security fixes label

Mar 5, 2023

bedevere-bot commented Mar 5, 2023

Copy link

Copy Markdown

GH-102439 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Mar 5, 2023


          pythongh-101979: argparse: fix a bug where parentheses in metavar arg…

97fde49

…ument of add_argument() were dropped (pythonGH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <[email protected]>

bedevere-bot removed the needs backport to 3.10 label

Mar 5, 2023

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Mar 5, 2023


          pythongh-101979: argparse: fix a bug where parentheses in metavar arg…

20dfd9a

…ument of add_argument() were dropped (pythonGH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <[email protected]>

miss-islington added a commit that referenced this pull request

Mar 5, 2023


          gh-101979: argparse: fix a bug where parentheses in metavar argument …

2a062f2

…of add_argument() were dropped (GH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <[email protected]>

miss-islington added a commit that referenced this pull request

Mar 5, 2023


          gh-101979: argparse: fix a bug where parentheses in metavar argument …

e748f9e

…of add_argument() were dropped (GH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <[email protected]>

yeojin-dev deleted the gh-101979 branch

March 5, 2023 22:40

hugovk pushed a commit to hugovk/cpython that referenced this pull request

Mar 6, 2023


          pythongh-101979: argparse: fix a bug where parentheses in metavar arg…

0e240a6

…ument of add_argument() were dropped (python#102318)

carljm added a commit to carljm/cpython that referenced this pull request

Mar 6, 2023


          Merge branch 'main' into nodeaddict

33dda2a

* main: (21 commits)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472)
  pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455)
  pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467)
  pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667)
  pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664)
  pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665)
  pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445)
  pythonGH-102341: Improve the test function for pow (python#102342)
  Fix unused classes in a typing test (pythonGH-102437)
  pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318)
  pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426)
  Move around example in to_bytes() to avoid confusion (python#101595)
  pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421)
  pythongh-96821: Add config option `--with-strict-overflow` (python#96823)
  pythongh-101992: update pstlib module documentation (python#102133)
  pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699)
  pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417)
  pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303)
  pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180)
  pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896)
  ...

carljm added a commit to carljm/cpython that referenced this pull request

Mar 7, 2023


          Merge branch 'main' into inlinecomp2

ecb313c

* main: (37 commits)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472)
  pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455)
  pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467)
  pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667)
  pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664)
  pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665)
  pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445)
  pythonGH-102341: Improve the test function for pow (python#102342)
  Fix unused classes in a typing test (pythonGH-102437)
  pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318)
  pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426)
  Move around example in to_bytes() to avoid confusion (python#101595)
  pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421)
  pythongh-96821: Add config option `--with-strict-overflow` (python#96823)
  pythongh-101992: update pstlib module documentation (python#102133)
  pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699)
  pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417)
  pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303)
  pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180)
  pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896)
  ...

This was referenced May 7, 2024

argparse usage should preserve () in metavars such as range(20) #62549

Closed

argparse.ArgumentParser.add_mutually_exclusive_group : metavar create parenthesis undefined behavior #89743

Closed

Reviewers

corona10

corona10 left review comments

JelleZijlstra

JelleZijlstra approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

JelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

yeojin-dev commented Feb 28, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corona10 commented Mar 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Mar 3, 2023

Choose a reason for hiding this comment

Uh oh!

yeojin-dev Mar 5, 2023

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra Mar 5, 2023

Choose a reason for hiding this comment

Uh oh!

yeojin-dev commented Mar 5, 2023

Uh oh!

bedevere-bot commented Mar 5, 2023

Uh oh!

miss-islington commented Mar 5, 2023

Uh oh!

bedevere-bot commented Mar 5, 2023

Uh oh!

bedevere-bot commented Mar 5, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yeojin-dev commented Feb 28, 2023 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.