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-30592: Fixed error messages for some builtins. #1996

Merged

Conversation

serhiy-storchaka
Copy link
Member

Error messages when pass keyword arguments to some builtins that
don't support keyword arguments contained double parenthesis: "()()".
The regression was intriduced by bpo-30534.

Error messages when pass keyword arguments to some builtins that
don't support keyword arguments contained double parenthesis: "()()".
The regression was intriduced by bpo-30534.
@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Jun 8, 2017
@serhiy-storchaka serhiy-storchaka merged commit 6cca5c8 into python:master Jun 8, 2017
@serhiy-storchaka serhiy-storchaka deleted the no-keywords-parens branch June 8, 2017 11:41
Copy link
Contributor

@SylvainDe SylvainDe left a comment

Choose a reason for hiding this comment

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

Looks good to me!
Do you reckon I'd make sense to have a unit test for a least one of these functions ?
Also, should we improve the _PyArg_NoKeywords documentation to mention that the provided function name is supposed to not have the parenthesis?

@serhiy-storchaka
Copy link
Member Author

This PR fixed 23 functions and methods. Tests for just one function wouldn't cover other cases, and adding 23 tests looks excessively. But adding tests for general cases which cover a lot of function is welcome.

Also, should we improve the _PyArg_NoKeywords documentation to mention that the provided function name is supposed to not have the parenthesis?

There is no documentation for _PyArg_NoKeywords except a comment before its definition. And since _PyArg_NoKeywords immediately follows a comment, I don't think that adding some words in a comment would add more clarity to the sources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants