ml_flag of nditer.close mismatches its implementation and documentation

[Elaphurus]

nditer.close (documentation) takes no arguments, the foreign function implements it also takes only the self parameter:

numpy/numpy/core/src/multiarray/nditer_pywrap.c

Lines 2317 to 2318 in 8ada030

	static PyObject *
	npyiter_close(NewNpyArrayIterObject *self)

However, this foreign function is declared with the ml_flag METH_VARARGS, which should be METH_NOARGS (Python/C API documentation):

numpy/numpy/core/src/multiarray/nditer_pywrap.c

Lines 2371 to 2372 in 8ada030

	{"close", (PyCFunction)npyiter_close,
	METH_VARARGS, NULL},

Otherwise, you can write it.close(1) or it.close('str') without getting any error reports.

[seberg]

PRs welcome. A very short test would be nice, but I think we can just change it since passing an argument can be a clear bug (or typo) if anyone actually does it.

Interestingly, if we would ever be using any non-standard calling convention, it would seem that we should also add the second argument PyObject *NPY_UNUSED(args)? Can we rely on cdecl calling convention? If its wrong, we are getting it wrong everywhere though, so its a different issue.

[Elaphurus]

NPY_UNUSED will only remove unused variable warnings and mangle the variable to avoid accidental use, as long as the ml_flag is not METH_NOARGS, you can always pass any number and type of arguments to the Python foreign function without getting an expected TypeError.

ndarray.__reduce__ can be an example:

numpy/numpy/core/src/multiarray/methods.c

Lines 1707 to 1708 in c1ce397

	static PyObject *
	array_reduce(PyArrayObject *self, PyObject *NPY_UNUSED(args))

numpy/numpy/core/src/multiarray/methods.c

Lines 2730 to 2732 in c1ce397

	{"__reduce__",
	(PyCFunction) array_reduce,
	METH_VARARGS, NULL},

Code like ndarray.__reduce__('str', 1) is allowed.

I think we should rely on ml_flag to indicate a certain calling convention.

When a foreign function is not declared with METH_NOARGS, there will be three cases that have the similar issues. That is when function implementation

• does not have the args parameter
• uses PyObject *NPY_UNUSED(args)
• uses just args as a parameter, but it is not actually used in the function body

[seberg]

Yes of course. The point is METH_NOARGS still passes args=NULL according to the documentation (and python source), so not adding PyObject *NPY_UNUSED(args) is technically incorrect.

Now if there were compilers/systems where that mattered, I am bet we would have noticed long ago (since it seems incorrect everywhere)... So its probably just OK and we can assume cdecl calling convention in C where it doesn't matter for the ABI when arguments are ignored or extra arguments passed (IIRC).

[mattip]

Closed by the PR listed above. Please reopen if I misunderstood or if there is more to do here.