Skip to content

Cannot use a python class as a default argument #2028

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

Closed
chatziko opened this issue Dec 15, 2019 · 2 comments · Fixed by #2317
Closed

Cannot use a python class as a default argument #2028

chatziko opened this issue Dec 15, 2019 · 2 comments · Fixed by #2317

Comments

@chatziko
Copy link

Issue description

Using any py::object as a default argument seems to be supported. But in the particular case when the object is a class, it fails at import type.

Reproducible example code

This works fine:

m.def(
    "test1",
    [](py::object a) { py::print(py::str(a)); },
    "a"_a = py::module::import("decimal").attr("__version__")
);

$ python
>>> import my_module;
>>> my_module.test1();
1.70

But this fails (only difference is Decimal in place of __version__)

m.def(
    "test2",
    [](py::object a) { py::print(py::str(a)); },
    "a"_a = py::module::import("decimal").attr("Decimal")
);

$ python
>>> import my_module;
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: TypeError: descriptor '__repr__' of 'decimal.Decimal' object needs an argument

I would expect to simply recieve the Decimal class in a, but it seems like pybind11 is trying to construct a Decimal object, or something like that.

@bstaletic
Copy link
Collaborator

This is a workaround:

#include <pybind11/pybind11.h>

namespace py = pybind11;
PYBIND11_MODULE(foo, m) {
	py::arg a_temp("a");
	auto arg = py::arg_v(a_temp, py::module::import("decimal").attr("Decimal"), "Decimal");
	m.def( "test1", [](py::object a) { py::print(py::str(a)); }, arg);
}

This happens because foo.__repr__() is not the same as repr(foo) and pybind is tripping over this here:

        for (auto &a: rec->args) {
            if (a.name)
                a.name = strdup(a.name);
            if (a.descr)
                a.descr = strdup(a.descr);
            else if (a.value)
                a.descr = strdup(a.value.attr("__repr__")().cast<std::string>().c_str());
        }

https://github.com/pybind/pybind11/blob/master/include/pybind11/pybind11.h#L225-L232

@bstaletic
Copy link
Collaborator

bstaletic commented Jul 23, 2020

I don't know if there's a better way to do this, but this patch seems to work.

diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index ee28705..93c6f95 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -228,12 +228,17 @@ protected:
            if (a.descr)
                 a.descr = strdup(a.descr);
            else if (a.value)
-                a.descr = strdup(a.value.attr("__repr__")().cast<std::string>().c_str());
+                a.descr = strdup(repr(a.value).cast<std::string>().c_str());
         }

         rec->is_constructor = !strcmp(rec->name, "__init__") || !strcmp(rec->name, "__setstate__");

EDIT: Update patch.

bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 23, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 23, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 23, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 24, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 24, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
bstaletic added a commit to bstaletic/pybind11 that referenced this issue Jul 24, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes pybind#2028
YannickJadoul pushed a commit that referenced this issue Jul 24, 2020
If the default argument value is a class, and not an instance of a
class, `a.value.attr("__repr__")` raises a `ValueError`. Switching to
`repr(a.value)` makes this use case work.

Fixes #2028
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants