wjakob / nanobind

nanobind: tiny and efficient C++/Python bindings
BSD 3-Clause "New" or "Revised" License
2.14k stars 161 forks source link

[BUG]: The typename defined in nb::sig found partly stripped in the generated stub #493

Closed a603032070 closed 3 months ago

a603032070 commented 3 months ago

Problem description

// foo.cpp
#include <nanobind/nanobind.h>
namespace nb = nanobind;
int add(int a) { return a; }
NB_MODULE(my_ext, m)
{
    m.def("add", &add, nb::sig("def add(a:my_ext.Bar, b:my_ext_b.XXX, c:my_ext_int)->my_ext_d.XXX"));
}

After compiling the above code, I got the stub file as below.

# my_ext.pyi
def add(a:Bar, b:b.XXX, c:my_ext_int)->d.XXX: ...

The module name my_ext got stripped from the typename of parameter a, b and d. IMO this behavior is not as desired. However, typename of param c (myextint) remains unchanged. The actual signature of function add doesn't matter.

Reproducible example code

// foo.cpp
#include <nanobind/nanobind.h>
namespace nb = nanobind;
int add(int a) { return a; }
NB_MODULE(my_ext, m)
{
    m.def("add", &add, nb::sig("def add(a:my_ext.Bar, b:my_ext_b.XXX, c:my_ext_int)->my_ext_d.XXX"));
}
wjakob commented 3 months ago

As far as I can tell, this isn't really a bug but simply a personal preference about how you wish the stubs to be rendered. Style isn't something up for discussion though: the stubs exist to be consumed by PyRight/PyLance and MyPy, and they work just fine with this style of type annotation. I will close this issue, but please feel free to add further information if this goes beyond a preference.

a603032070 commented 3 months ago

I expected that nb::sig offers a way to fully control the stub. There are 2 modules using nanobind in my own project, say module "my_ext" and "my_ext_structs". Functions bound in "my_ext" may have parameters of some type T. i.e. void do(T t) { ... } m.def("do", &do) And T is bound in module "my_ext_structs". nb::class_<T>(m, "T") The default stub of "my_ext" would be something like def do(t:T)->None: ... while it's desired to be def do(t:my_ext_structs.T)->None: ... So I turned to nb::sig, ~m.def("do", &do)~ m.def("do", &do, nb::sig("def do(t:my_ext_structs.T)->None")) and the stub became: def do(t:structs.T)->None: ...

wjakob commented 3 months ago

I don't understand. The shortening to leave out the module name will only occur for references within that same module, where it is unambiguous that T refers to the declaration of T within the same module. If you have a function that uses T from the other module, then stubgen will leave the fully qualified typename and/or import the type from there.

wjakob commented 3 months ago

If you are convinced that there is an issue, then my recommendation is that you make a simple and short reproducer by forking the nanobind_example github repository.

a603032070 commented 3 months ago

If you are convinced that there is an issue, then my recommendation is that you make a simple and short reproducer by forking the nanobind_example github repository.

https://github.com/a603032070/nanobind_example I've reproduced it in my fork. I believe the implementation in nanobind.stubgen may not handle my case, and I showed a way to patch it in the reproduce.py.