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

MAINT: Include the function name in all argument error messages #8861

Merged
merged 1 commit into from
Mar 28, 2017

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Mar 28, 2017

Performed using the following regex, replacing with $0:$1. Some manual cleanup
was then required

^[a-z]+_(\w+)(?:(?!\}).*\n)+.*PyArg_ParseTuple(AndKeywords)?\(args[^"]+"[^:"]+(?=")

@@ -3263,7 +3263,7 @@ array_can_cast_safely(PyObject *NPY_UNUSED(self), PyObject *args,
NPY_CASTING casting = NPY_SAFE_CASTING;
static char *kwlist[] = {"from", "to", "casting", NULL};

if(!PyArg_ParseTupleAndKeywords(args, kwds, "OO&|O&", kwlist,
if(!PyArg_ParseTupleAndKeywords(args, kwds, "OO&|O&:can_cast", kwlist,
Copy link
Member Author

Choose a reason for hiding this comment

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

It's a little annoying how sometimes the C and python function names are inconsistent, like here (can_cast vs can_cast_safely)

@@ -2855,7 +2855,7 @@ array_fastCopyAndTranspose(PyObject *NPY_UNUSED(dummy), PyObject *args)
{
PyObject *a0;

if (!PyArg_ParseTuple(args, "O", &a0)) {
if (!PyArg_ParseTuple(args, "O:fastCopyAndTranspose", &a0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is _fastCopyAndTranspose

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

@juliantaylor
Copy link
Contributor

looks good modulo one nitpick

while greatly appreciated you should slow down a little with the changes to avoid a too fast burnout, also we don't have enough review time to keep up.

Performed using the following regex, replacing with $0:$1. Some manual cleanup
was then required.

    ^[a-z]+_(\w+)(?:(?!\}).*\n)+.*PyArg_ParseTuple(AndKeywords)?\(args[^"]+"[^:"]+(?=")
@charris
Copy link
Member

charris commented Mar 28, 2017

you should slow down a little with the changes

My motto, "Gather ye rosebuds while ye may." Few last more than a few years in open source, the needs of life eventually take precedence...

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.

None yet

3 participants