8000 gh-126068: Fix exceptions in the argparse module by serhiy-storchaka · Pull Request #126069 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-126068: Fix exceptions in the argparse module #126069

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

Merged
merged 5 commits into from
Oct 30, 2024

Conversation

serhiy-storchaka
Copy link
Member
@serhiy-storchaka serhiy-storchaka commented Oct 28, 2024
  • Only error messages for ArgumentError and ArgumentTypeError are now translated.
  • ArgumentError is now only used for command line errors, not for logical errors in the program.

* Only error messages for ArgumentError and ArgumentTypeError are now
  translated.
* ArgumentError is now only used for command line errors, not for logical
  errors in the program.
Copy link
Member
@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Are we sure that this wouldn't break code in the wild that catch those? ArgumentError is a public exception (exposed in __all__) so raising ValueError instead might break code that have except ArgumentError (if they looked at the source code).

I don't mind not translating them, but I'm not very comfortable with changing them like that without a more visible changelog (namely, by adding a true What's New entry).

@serhiy-storchaka
Copy link
Member Author

I am sure that this will not break working program. ArgumentError is an exception for command line errors, it is not appropriate for logical errors. The working program should not contain logical errors, so it would not be affected.

Copy link
Member
@tomasr8 tomasr8 left a comment

Choose a reason for hiding this comment

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

LGTM! I agree these error messages should not be i18n'd.

Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
Copy link
Member
@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. Just a couple comments!

serhiy-storchaka and others added 2 commits October 30, 2024 01:07
Co-authored-by: Savannah Ostrowski <savannahostrowski@gmail.com>
@serhiy-storchaka
Copy link
Member Author

On other hand, if we change types of exceptions, we can also change some ValueError to TypeError when the latter is more appropriate -- when a required argument is missed or duplicated, or incorrect type of argument is passed. TypeError is already used for some of such errors, and it is also raised implicitly for such types of errors.

@serhiy-storchaka
Copy link
Member Author

Thank you for your review @savannahostrowski. I reverted changes to the NEWS file, because we should use full qualified names of exceptions in this case, and it will be too verbose. There are already links to the argparse module, so new links will not add much value.

@serhiy-storchaka serhiy-storchaka merged commit cc9a183 into python:main Oct 30, 2024
36 checks passed
@serhiy-storchaka serhiy-storchaka deleted the argparse-errors branch October 30, 2024 16:14
@picnixz
Copy link
Member
picnixz commented Oct 30, 2024

, because we should use full qualified names of exceptions in this case, and it will be too verbose

FYI: you can use ~ and make it render as ArgumentError:

:exc:`~argparse.ArgumentError`

@serhiy-storchaka
Copy link
Member Author

Yes, of course. But this is much noise for little benefit.

picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
* Only error messages for ArgumentError and ArgumentTypeError are now
  translated.
* ArgumentError is now only used for command line errors, not for logical
  errors in the program.
* TypeError is now raised instead of ValueError for some logical errors.
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
* Only error messages for ArgumentError and ArgumentTypeError are now
  translated.
* ArgumentError is now only used for command line errors, not for logical
  errors in the program.
* TypeError is now raised instead of ValueError for some logical errors.
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.

4 participants
0