Skip to content

bpo-30592: Fixed error messages for some builtins. #1996

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.

Sign up for GitHub

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

Jump to bottom
Merged
serhiy-storchaka merged 1 commit into from
Jun 8, 2017
Merged

bpo-30592: Fixed error messages for some builtins. #1996

serhiy-storchaka merged 1 commit into from
Jun 8, 2017

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jun 8, 2017

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
bpo-30592: Fixed error messages for some builtins.
65dfc6a 
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
SylvainDe
SylvainDe reviewed Jun 8, 2017
View reviewed changes
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

serhiy-storchaka commented Jun 8, 2017

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

Reviewers

1 more reviewer

@SylvainDe SylvainDe SylvainDe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@serhiy-storchaka @SylvainDe @the-knights-who-say-ni