fix: Reject keyword argument None with .none(false) (#2611)

* demo kwarg with none(false)

* Reorder and extend tests for arg::none(false) in test_methods_and_attributes.py::test_accepts_none

* Fix arg::none() for keyword arguments

* Add changelog note

* Fix names of no_none_kw test functions

Co-authored-by: Yannick Jadoul <yannick.jadoul@belgacom.net>
This commit is contained in:
Mana Borwornpadungkitti 2020-10-20 17:57:22 -04:00 committed by GitHub
parent 7f9445a626
commit 6edd0e6d90
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 11 deletions

View File

@ -204,6 +204,9 @@ Smaller or developer focused features and fixes:
* Avoid a segfault on some compilers when types are removed in Python. * Avoid a segfault on some compilers when types are removed in Python.
`#2564 <https://github.com/pybind/pybind11/pull/2564>`_ `#2564 <https://github.com/pybind/pybind11/pull/2564>`_
* ``py::arg::none()`` is now also respected when passing keyword arguments.
`#2611 <https://github.com/pybind/pybind11/pull/2611>`_
* PyPy fixes, PyPy 7.3.x now supported, including PyPy3. (Known issue with * PyPy fixes, PyPy 7.3.x now supported, including PyPy3. (Known issue with
PyPy2 and Windows `#2596 <https://github.com/pybind/pybind11/issues/2596>`_). PyPy2 and Windows `#2596 <https://github.com/pybind/pybind11/issues/2596>`_).
`#2146 <https://github.com/pybind/pybind11/pull/2146>`_ `#2146 <https://github.com/pybind/pybind11/pull/2146>`_

View File

@ -606,15 +606,15 @@ protected:
// 1.5. Fill in any missing pos_only args from defaults if they exist // 1.5. Fill in any missing pos_only args from defaults if they exist
if (args_copied < func.nargs_pos_only) { if (args_copied < func.nargs_pos_only) {
for (; args_copied < func.nargs_pos_only; ++args_copied) { for (; args_copied < func.nargs_pos_only; ++args_copied) {
const auto &arg = func.args[args_copied]; const auto &arg_rec = func.args[args_copied];
handle value; handle value;
if (arg.value) { if (arg_rec.value) {
value = arg.value; value = arg_rec.value;
} }
if (value) { if (value) {
call.args.push_back(value); call.args.push_back(value);
call.args_convert.push_back(arg.convert); call.args_convert.push_back(arg_rec.convert);
} else } else
break; break;
} }
@ -628,11 +628,11 @@ protected:
bool copied_kwargs = false; bool copied_kwargs = false;
for (; args_copied < num_args; ++args_copied) { for (; args_copied < num_args; ++args_copied) {
const auto &arg = func.args[args_copied]; const auto &arg_rec = func.args[args_copied];
handle value; handle value;
if (kwargs_in && arg.name) if (kwargs_in && arg_rec.name)
value = PyDict_GetItemString(kwargs.ptr(), arg.name); value = PyDict_GetItemString(kwargs.ptr(), arg_rec.name);
if (value) { if (value) {
// Consume a kwargs value // Consume a kwargs value
@ -640,14 +640,18 @@ protected:
kwargs = reinterpret_steal<dict>(PyDict_Copy(kwargs.ptr())); kwargs = reinterpret_steal<dict>(PyDict_Copy(kwargs.ptr()));
copied_kwargs = true; copied_kwargs = true;
} }
PyDict_DelItemString(kwargs.ptr(), arg.name); PyDict_DelItemString(kwargs.ptr(), arg_rec.name);
} else if (arg.value) { } else if (arg_rec.value) {
value = arg.value; value = arg_rec.value;
}
if (!arg_rec.none && value.is_none()) {
break;
} }
if (value) { if (value) {
call.args.push_back(value); call.args.push_back(value);
call.args_convert.push_back(arg.convert); call.args_convert.push_back(arg_rec.convert);
} }
else else
break; break;

View File

@ -336,6 +336,9 @@ TEST_SUBMODULE(methods_and_attributes, m) {
m.def("ok_none4", &none4, py::arg().none(true)); m.def("ok_none4", &none4, py::arg().none(true));
m.def("ok_none5", &none5); m.def("ok_none5", &none5);
m.def("no_none_kwarg", &none2, py::arg("a").none(false));
m.def("no_none_kwarg_kw_only", &none2, py::kw_only(), py::arg("a").none(false));
// test_str_issue // test_str_issue
// Issue #283: __str__ called on uninitialized instance when constructor arguments invalid // Issue #283: __str__ called on uninitialized instance when constructor arguments invalid
py::class_<StrIssue>(m, "StrIssue") py::class_<StrIssue>(m, "StrIssue")

View File

@ -404,6 +404,19 @@ def test_accepts_none(msg):
assert m.ok_none4(None) == -1 assert m.ok_none4(None) == -1
assert m.ok_none5(None) == -1 assert m.ok_none5(None) == -1
with pytest.raises(TypeError) as excinfo:
m.no_none_kwarg(None)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.no_none_kwarg(a=None)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.no_none_kwarg_kw_only(None)
assert "incompatible function arguments" in str(excinfo.value)
with pytest.raises(TypeError) as excinfo:
m.no_none_kwarg_kw_only(a=None)
assert "incompatible function arguments" in str(excinfo.value)
def test_str_issue(msg): def test_str_issue(msg):
"""#283: __str__ called on uninitialized instance when constructor arguments invalid""" """#283: __str__ called on uninitialized instance when constructor arguments invalid"""