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.

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

bpo-37880: for argparse add_argument with action='store_const', const defaults to None. #26707

Merged
merged 7 commits into from
Jul 31, 2021

Conversation

jdevries3133
Copy link
Contributor

@jdevries3133 jdevries3133 commented Jun 13, 2021

This PR implements changes to argparse as per bpo-37880. Updates to documentation, tests, and news are also included. I'm unable to run all regrtests on my system right now due to bpo-44411, but test_argparse is passing, of course.

https://bugs.python.org/issue37880

@jdevries3133
Copy link
Contributor Author

@jacobtylerwalls thank you! I fixed it. That freaking word gets me every time.

Comment on lines 980 to 984
If ``const`` is not provided to :meth:`~ArgumentParser.add_argument`, it
will receive a default value of ``None``.

.. versionchanged:: 3.10
``const=None`` by default.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be one level higher to clarify it doesn't just apply to store_const and append_const? The part being removed below made that more clear.

If so, then the versionchanged could be more specific about what was changed in 3.11. (BTW 3.11 is under dev now)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will take another pass at the documentation, because I'm seeing a few rough edges. However, to be clear, the default assignment of None to the const kwarg is only new behavior when the action type is append_const or store_const. All other action types always assigned the default value of None to const. This PR just makes the behavior consistent when the action type is append_const or store_const, so the change in behavior is limited in scope.

At the same time, I think that sticking the :versionchanged: note under the bullet point like I did initially just plain looks wrong, so I will bring the note back to its original position.

As far as "other rough edges":

  • In the action section:
    • Grammatical change: exchanging parenthesis for a semicolon where there were two separate clauses in the action='append_const' section.
    • Add a brief note about the default assignment of const=None in the action='store_const' section.
  • Add a paragraph break in the name or flags section. This makes it much easier for the reader to draw contrast between these two examples at a glance.

@vsajip
Copy link
Member

vsajip commented Jul 28, 2021

Closing and reopening to run checks again.

@vsajip vsajip closed this Jul 28, 2021
@vsajip vsajip reopened this Jul 28, 2021
@vsajip
Copy link
Member

vsajip commented Jul 29, 2021

Closing and reopening to run checks yet again ...

@vsajip vsajip closed this Jul 29, 2021
@vsajip vsajip reopened this Jul 29, 2021
@vsajip vsajip merged commit 0ad1732 into python:main Jul 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants