From 07a6f0c298dfab554357d62b6beb5cab20ecbe54 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 18 Oct 2024 10:22:05 +0200 Subject: [PATCH 1/3] Arguments for nrnpy_pyCallObject are created with nanobind (#3128) --- src/nrnpython/nrnpy_p2h.cpp | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 6bfae26924..ea541b8f93 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -177,28 +177,22 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { PyErr_Print(); hoc_execerror("No attribute:", sym->name); } - PyObject* args = 0; Object* on; PyObject* result = 0; if (isfunc) { - args = PyTuple_New(nindex); + nb::list args{}; for (i = 0; i < nindex; ++i) { PyObject* arg = nrnpy_hoc_pop("isfunc py2n_component"); if (!arg) { PyErr2NRNString e; e.get_pyerr(); - Py_DECREF(args); hoc_execerr_ext("arg %d error: %s", i, e.c_str()); } - // PyObject_Print(arg, stdout, 0); - // printf(" %d arg %d\n", arg->ob_refcnt, i); - if (PyTuple_SetItem(args, nindex - 1 - i, arg)) { - assert(0); - } + args.append(nb::borrow(arg)); } + args.reverse(); // printf("PyObject_CallObject %s %p\n", sym->name, tail); - result = nrnpy_pyCallObject(nb::borrow(tail), nb::borrow(args)).release().ptr(); - Py_DECREF(args); + result = nrnpy_pyCallObject(nb::borrow(tail), args).release().ptr(); // PyObject_Print(result, stdout, 0); // printf(" result of call\n"); if (!result) { @@ -216,7 +210,7 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { return; } } else if (nindex) { - PyObject* arg; + nb::object arg; int n = hoc_pop_ndim(); if (n > 1) { hoc_execerr_ext( @@ -226,15 +220,15 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { n); } if (hoc_stack_type() == NUMBER) { - arg = Py_BuildValue("l", (long) hoc_xpop()); + arg = nb::int_((long) hoc_xpop()); } else { // I don't think it is syntactically possible // for this to be a VAR. It is possible for it to // be an Object but the GetItem below will raise // TypeError: list indices must be integers or slices, not hoc.HocObject - arg = nrnpy_hoc_pop("nindex py2n_component"); + arg = nb::borrow(nrnpy_hoc_pop("nindex py2n_component")); } - result = PyObject_GetItem(tail, arg); + result = PyObject_GetItem(tail, arg.ptr()); if (!result) { PyErr_Print(); hoc_execerror("Python get item failed:", hoc_object_name(ob)); @@ -468,24 +462,17 @@ static double func_call(Object* ho, int narg, int* err) { nb::callable po = nb::borrow(((Py2Nrn*) ho->u.this_pointer)->po_); nanobind::gil_scoped_acquire lock{}; - PyObject* args = PyTuple_New((Py_ssize_t) narg); - if (args == NULL) { - hoc_execerror("PyTuple_New failed", 0); - } + nb::list args{}; for (int i = 0; i < narg; ++i) { PyObject* item = nrnpy_hoc_pop("func_call"); if (item == NULL) { - Py_XDECREF(args); hoc_execerror("nrnpy_hoc_pop failed", 0); } - if (PyTuple_SetItem(args, (Py_ssize_t) (narg - i - 1), item) != 0) { - Py_XDECREF(args); - hoc_execerror("PyTuple_SetItem failed", 0); - } + args.append(nb::borrow(item)); } + args.reverse(); - nb::object r = nrnpy_pyCallObject(po, nb::borrow(args)); - Py_XDECREF(args); + nb::object r = nrnpy_pyCallObject(po, args); double rval = 0.0; if (!r.is_valid()) { if (!err || *err) { From b9f5af1739b1a12c871703024bea39d9521b51ec Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Mon, 21 Oct 2024 18:37:21 +0200 Subject: [PATCH 2/3] steal instead of borrow --- src/nrnpython/nrnpy_p2h.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index ea541b8f93..57bed4e4f4 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -182,13 +182,13 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { if (isfunc) { nb::list args{}; for (i = 0; i < nindex; ++i) { - PyObject* arg = nrnpy_hoc_pop("isfunc py2n_component"); + nb::object arg = nb::steal(nrnpy_hoc_pop("isfunc py2n_component")); if (!arg) { PyErr2NRNString e; e.get_pyerr(); hoc_execerr_ext("arg %d error: %s", i, e.c_str()); } - args.append(nb::borrow(arg)); + args.append(arg); } args.reverse(); // printf("PyObject_CallObject %s %p\n", sym->name, tail); From 4819f04df6e3b348b3fdd234ee1abc26d51a94e0 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 23 Oct 2024 14:12:01 +0200 Subject: [PATCH 3/3] Always steal --- src/nrnpython/nrnpy_p2h.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 57bed4e4f4..c9dc98db92 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -226,7 +226,7 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { // for this to be a VAR. It is possible for it to // be an Object but the GetItem below will raise // TypeError: list indices must be integers or slices, not hoc.HocObject - arg = nb::borrow(nrnpy_hoc_pop("nindex py2n_component")); + arg = nb::steal(nrnpy_hoc_pop("nindex py2n_component")); } result = PyObject_GetItem(tail, arg.ptr()); if (!result) { @@ -464,11 +464,11 @@ static double func_call(Object* ho, int narg, int* err) { nb::list args{}; for (int i = 0; i < narg; ++i) { - PyObject* item = nrnpy_hoc_pop("func_call"); - if (item == NULL) { + nb::object item = nb::steal(nrnpy_hoc_pop("func_call")); + if (!item) { hoc_execerror("nrnpy_hoc_pop failed", 0); } - args.append(nb::borrow(item)); + args.append(item); } args.reverse();