GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-33109: argparse subparsers are once again not required by default by ned-deily · Pull Request #6919 · python/cpython

Conversation

@ned-deily

@ned-deily ned-deily commented May 16, 2018 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

bpo-26510 in 3.7.0a2 changed the behavior of argparse to make
subparsers required by default, returning to the behavior of 2.7
and 3.2. The behavior was changed in 3.3 to be no longer required.
While it might make more sense to have the default to required,
compatibility with 3.3 through 3.6 is probably less disruptive
than trying to reintroduce compatibility with 2.7 at this point.
This change restores the 3.6 behavior.

https://bugs.python.org/issue33109

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ned-deily
bpo-33109: argparse subparsers are once again not required by default
cbea1d1 
bpo-26510 in 3.7.0a2 changed the behavior of argparse to make
subparsers required by default, returning to the behavior of 2.7
and 3.2. The behavior was changed in 3.3 to be no longer required.
While it might make more sense to have the default to required,
compatibility with 3.3 through 3.6 is probably less disruptive
than trying to reintroduce compatibility with 2.7 at this point.
This change restores the 3.6 behavior.
@ned-deily ned-deily added the needs backport to 3.7 label May 16, 2018
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 16, 2018
@bedevere-bot bedevere-bot added the awaiting merge label May 16, 2018
@asottile asottile mentioned this pull request May 16, 2018
bpo-26510: Allow argparse add_subparsers to take required kwarg. #3027
Merged
@asottile

asottile commented May 16, 2018

Copy link
Copy Markdown
Contributor

I still think we should consider the new behaviour more correct and the 3.3-3.6 behaviour a bug. This reintroduces the bug.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ned-deily

ned-deily commented May 16, 2018

Copy link
Copy Markdown
Member Author

@asottile Understood but see the discussion in https://bugs.python.org/issue33109 as to why we agree that compatibility with 3.6 is more important. Counterarguments welcome there!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed May 22, 2018
View reviewed changes
Comment thread Lib/argparse.py
parser_class,
dest=SUPPRESS,
required=True,
required=False,

@serhiy-storchaka serhiy-storchaka May 22, 2018

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.

While we are here, please move required to the end of the list of parameters. It was inserted in the middle of non-keyword-only parameters.

Sorry, something went wrong.

Uh oh!

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

@asottile asottile May 22, 2018

Copy link
Copy Markdown
Contributor

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.

This was discussed here

It makes no difference because add_subparsers does not take positional arguments (all arguments are keyword-only). The ordering here was chosen to be consistent with the rest of the classes in the module

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
JonathonReinhart reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@serhiy-storchaka serhiy-storchaka May 22, 2018

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.

Oh, sorry. You are right.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
asottile reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@ned-deily
ned-deily merged commit 8ebf5ce into python:master May 24, 2018
@miss-islington

miss-islington commented May 24, 2018

Copy link
Copy Markdown
Contributor

Thanks @ned-deily for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting merge label May 24, 2018
@ned-deily
ned-deily deleted the bpo-33109 branch May 24, 2018 01:55
@bedevere-bot

bedevere-bot commented May 24, 2018

Copy link
Copy Markdown

GH-7089 is a backport of this pull request to the 3.7 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.7 label May 24, 2018
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 24, 2018
@ned-deily @miss-islington
bpo-33109: argparse subparsers are once again not required by default (…
2538eb3 
…pythonGH-6919)

bpo-26510 in 3.7.0a2 changed the behavior of argparse to make
subparsers required by default, returning to the behavior of 2.7
and 3.2. The behavior was changed in 3.3 to be no longer required.
While it might make more sense to have the default to required,
compatibility with 3.3 through 3.6 is probably less disruptive
than trying to reintroduce compatibility with 2.7 at this point.
This change restores the 3.6 behavior.
(cherry picked from commit 8ebf5ce)

Co-authored-by: Ned Deily <[email protected]>
ned-deily added a commit that referenced this pull request May 24, 2018
@miss-islington @ned-deily
bpo-33109: argparse subparsers are once again not required by default (…
dd7a255 
…GH-6919) (GH-7089)

bpo-26510 in 3.7.0a2 changed the behavior of argparse to make
subparsers required by default, returning to the behavior of 2.7
and 3.2. The behavior was changed in 3.3 to be no longer required.
While it might make more sense to have the default to required,
compatibility with 3.3 through 3.6 is probably less disruptive
than trying to reintroduce compatibility with 2.7 at this point.
This change restores the 3.6 behavior.
(cherry picked from commit 8ebf5ce)

Co-authored-by: Ned Deily <[email protected]>
@smcv smcv mentioned this pull request Jun 18, 2018
bmaptools/CLI.py: Print help in case of no arguments intel/bmap-tools#51
Closed
@Mariatta Mariatta mentioned this pull request Jul 15, 2018
Test against 3.7 dev on travis ci pypa/flit#188
Closed
@merwok merwok mentioned this pull request Oct 4, 2019
bpo-26510: Add versionchanged for required arg of add_subparsers #16588
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
+1 more reviewer
@asottile asottile asottile left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

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.

6 participants

@ned-deily @asottile @miss-islington @bedevere-bot @serhiy-storchaka @the-knights-who-say-ni
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.