sizmailov / pybind11-stubgen

Generate stubs for python modules
Other
238 stars 50 forks source link

Enums as default value make signature degrade to (*args, **kwargs) -> typing.Any #50

Closed marcoffee closed 4 years ago

marcoffee commented 4 years ago

When I try to use an (previously defined via pybind11::enum_) enum as default value for a pybind11::arg, pybind11-stubgen thinks it is a C++ type. Below is an example:

#include <pybind11/pybind11.h>

namespace py = pybind11;

enum TestEnum : int8_t {
  VALUE_1 = 0,
  VALUE_2 = 1,
  VALUE_3 = 2
};

PYBIND11_MODULE(test_module, m) {
  py::enum_<TestEnum>(m, "TestEnum")
    .value("VALUE_1", TestEnum::VALUE_1)
    .value("VALUE_2", TestEnum::VALUE_2)
    .value("VALUE_3", TestEnum::VALUE_3);

  m.def(
    "test_function",
    [] (TestEnum enum_val = TestEnum::VALUE_1) {},
    py::arg("enum_val") = TestEnum::VALUE_1
  );
}

After installing it as sdutil.test_module via pip and running pybind11-stubgen sdutil.test_module, I get the following errors:

[2020-10-13 23:25:26,067] {__init__.py:95} ERROR - Generated stubs signature is degraded to `(*args, **kwargs) -> typing.Any` for
[2020-10-13 23:25:26,067] {__init__.py:99} ERROR - def test_function(enum_val: sdutil.test_module.TestEnum = <TestEnum.VALUE_1: 0>) -> None: ...
[2020-10-13 23:25:26,067] {__init__.py:100} ERROR -                                                           ^-- Invalid syntax
[2020-10-13 23:25:26,067] {__init__.py:918} INFO - Useful link: Avoiding C++ types in docstrings:
[2020-10-13 23:25:26,067] {__init__.py:919} INFO -       https://pybind11.readthedocs.io/en/master/advanced/misc.html#avoiding-cpp-types-in-docstrings

When I cast TestEnum::VALUE_1 to int8_t, the stubs are generated, but when I call the function without enum_val parameter, pybind11 complains that the type is invalid.

Thanks in advance!

sizmailov commented 4 years ago

The error message here is not precise. You have bad default value in signature and this triggers the error message:

def test_function(enum_val: sdutil.test_module.TestEnum = <TestEnum.VALUE_1: 0>) -> None

Since https://github.com/pybind/pybind11/pull/2126 the repr() returns <>-enclosed string which is not valid python expression and can't be used as default value. You can fix it by providing custom representation of default value via py::arg_v [doc] or (re-)defining repr/str for enum class.

sizmailov commented 4 years ago

Note: your binding code is correct from runtime perspective, pybind11-stubgen complaints only about docstrings it finds within the module.

marcoffee commented 4 years ago

Thanks for the help! Just for the record, I simply replaced the line py::arg("enum_val") = TestEnum::VALUE_1 with py::arg_v("enum_val", TestEnum::VALUE_1, "TestEnum.VALUE_1") and now it works fine. I also found a way to circumvent some of those cases by adapting the function replace_default_pybind11_repr_with_ellipses. Can I create a pull request for this fix?

sizmailov commented 4 years ago

IMO binding code should to be as correct as possible, including generated docstrings. py::arg_v is the right tool to fix default value representation in signatures. While pybind11-stubgen can apply some fix-ups it's a non-goal of the project.

PRs are well appreciated, but I think here I have a patch (https://github.com/sizmailov/pybind11-stubgen/commit/311b8e6f7a51189559134b0d9002a8f77fe4e2d6) which does what you've planned.

The patch test reveals another issue https://github.com/sizmailov/pybind11-stubgen/commit/311b8e6f7a51189559134b0d9002a8f77fe4e2d6#r43237564, but I think I'll merge it anyway.

marcoffee commented 4 years ago

I think that this patch is slightly better than my solution (b1a4b1b1fd7c77ebb18cfec6347032f2845e1d51), as it avoids searching the string twice. In addition, my solution also reveals this problem. If you need any help with this matter, let me know.