Remove redundant if check from argparser file.#8766
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Remove redundant if check from optional argument function in argparser.
the-knights-who-say-ni commented Aug 14, 2018
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. When your account is ready, please add a comment in this pull request You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
sp1rs commented Aug 14, 2018
signed the CLA, Please remove the |
| option_strings.append(option_string) | ||
| ifoption_string[0] inself.prefix_chars: | ||
| iflen(option_string) >1: | ||
| ifoption_string[1] inself.prefix_chars: |
There was a problem hiding this comment.
I don't think these checks are redundant.
Consider self.prefix_chars ={'-'}
The left hand side requires '--foo' whereas your updated code incorrectly allows X-foo
I do think these conditions could be combined, perhaps something like:
if ( option_string[0] inself.prefix_charsandlen(option_string) >1andoption_string[1] inself.prefix_chars ): ...There was a problem hiding this comment.
Please check the code from line no 1463. It checks for the condition option_string[0] in self.prefix_chars.
There was a problem hiding this comment.
Ah indeed! Maybe use elif so that's more clear at a glance?
There was a problem hiding this comment.
I can do this.
if not option_string[0] in self.prefix_chars: # Some code. raise else: option_strings.append(option_string) if len(option_string) > 1 and option_string[1] in self.prefix_chars: long_option_strings.append(option_string) Or the current code is also fine.
There was a problem hiding this comment.
@asottile This change looks good. No need for else. This looks cleaner. Please review.
asottile left a comment
asottile left a comment There was a problem hiding this comment.
yeah you;re right, seems fine -- didn't notice the statement in the middle so elif would be way more work than necessary.
sp1rs commented Aug 24, 2018
@asottile How and when will this Pull request be approved and merged to master? |
asottile commented Aug 24, 2018
No idea, I'm just a rando |
sp1rs commented Nov 29, 2018
@bedevere-bot Please someone review. |
sp1rs commented Jun 20, 2019
@benjaminp@merwok Please review the PR. |
* master: (599 commits) Docs: Improved phrasing (pythonGH-14069) Remove redundant if check from optional argument function in argparse. (pythonGH-8766) bpo-37289: Add a test for if with ifexpr in the peephole optimiser to detect regressions (pythonGH-14127) Update What's New in Python 3.9 (pythonGH-14253) bpo-36511: Improve ARM32 buildbot scripts (pythonGH-14251) bpo-37151: remove _PyCFunction_FastCallDict (pythonGH-14269) Fix typo, 'widger' -> 'widget', in idlelib/tree.py (pythonGH-14263) Fix bpo number in News file. (pythonGH-14260) bpo-37342: Fix the incorrect nb_index's type in typeobj documentation (pythonGH-14241) Update What's New in Python 3.8 (pythonGH-14239) bpo-36710: Use tstate in pylifecycle.c (pythonGH-14249) Add missing single quote in io.TextIOWrapper.reconfigure documentation (pythonGH-14246) bpo-36511: Add buildbot scripts and fix tests for Windows ARM32 buildbot (pythonGH-13454) bpo-37333: Ensure IncludeTkinter has a value (pythonGH-14240) bpo-37331: Clarify format of socket handler messages in the documentation. (pythonGH-14234) bpo-37258: Not a bug, but added a unit test and updated documentation. (pythonGH-14229) bpo-36710: Remove PyImport_Cleanup() function (pythonGH-14221) Fix name of '\0'. (pythonGH-14222) bpo-36710: Add tstate parameter in import.c (pythonGH-14218) Document typing.ForwardRef (pythonGH-14216) ...
6 participants
Remove redundant if check from optional argument function in argparser.