Skip to content

gh-137786: fix error msg for *args, **kwargs default value#137793

Closed
hnnynh wants to merge 2 commits into
python:mainfrom
hnnynh:gh-137786
Closed

gh-137786: fix error msg for *args, **kwargs default value#137793
hnnynh wants to merge 2 commits into
python:mainfrom
hnnynh:gh-137786

Conversation

@hnnynh

@hnnynh hnnynh commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

Comment thread Lib/test/test_syntax.py Outdated
... pass
Traceback (most recent call last):
SyntaxError: var-positional parameter cannot have default value
SyntaxError: var-positional parameter's default value cannot be changed because it has the immutable default value ()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is clearer. "var-positional parameter cannot have default value" is oddly worded but I think equal or better to this proposal.

Comment thread Lib/test/test_syntax.py Outdated
... pass
Traceback (most recent call last):
SyntaxError: var-keyword parameter cannot have default value
SyntaxError: var-keyword parameter's default value cannot be changed because it has the immutable default value {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above, but this is worse -- it confuses the reader over {} (which is not immutable) vs the default (which notionally is).

@AA-Turner AA-Turner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of adding "it has immutable default X", I would focus on the start of the error message -- we could use friendlier terms than "var-positional" or "var-keyword". I think the changes to the end of the message are unhelpful at best just as they make it longer.

@bedevere-app

bedevere-app Bot commented Aug 15, 2025

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@skirpichev

Copy link
Copy Markdown
Member

@AA-Turner, see issue thread. Could you explain why there is an issue at all?

Comment thread Lib/test/test_syntax.py Outdated
>>> lambda a,*b=3,c: None
Traceback (most recent call last):
SyntaxError: var-positional parameter cannot have default value
SyntaxError: var-positional parameter's default value cannot be changed because it has the immutable default value ()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
SyntaxError: var-positional parameter's default value cannot be changed because it has the immutable default value ()
SyntaxError: Cannot change the default value of a var-positional parameter because it is already defined.

@AA-Turner Maybe this would make more sense to people?
For me, “immutable default value” feels too detailed, and the current message isn’t very user-friendly.

@hnnynh

hnnynh commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

I agree with the comment that the original error messages are correct, but I'd like to suggest adding explanations that are easy for users to understand.
So I have updated the messages. Could you please review it?

@skirpichev

Copy link
Copy Markdown
Member

@hnnynh, please avoid force-pushes. Don't alter git history.

@hnnynh

hnnynh commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

@hnnynh, please avoid force-pushes. Don't alter git history.

I made force-pushes just to update the message. I’ll ensure future changes are committed properly.

@skirpichev

Copy link
Copy Markdown
Member

I made force-pushes just to update the message.

Just add a new commit. All commits will be squashed on merge. If you wish to suggest core developers the final commit message - please edit pr title/description.

@hnnynh

hnnynh commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

The issue is closed, so I will close the PR as well. Thank you for discussing this with me. And I appreciate your attentions and comments.

@hnnynh hnnynh closed this Aug 15, 2025
@corona10

Copy link
Copy Markdown
Member

According to #137786 (comment), it might not worth to update message :) Thanks for all efforts @hnnynh , looking forward other PRs in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants