Fix stubgen prefixing issue

[Ahajha]

Situation: There are two submodules at the same "level", and the name of one of them is a prefix of the name of the other. When stubgen renders the types coming from the longer one in the shorter one, it naively checks that one is a prefix of the other, and ends up stripping away the prefix plus one extra character. I presume this is intended to be a submodule check.

Repro:

#include "nanobind/nanobind.h"
namespace nb = nanobind;

struct Type {};

NB_MODULE(_core, m) {
  nb::class_<Type>(m.def_submodule("mosh"), "Type");
  m.def_submodule("mo").def("func", [] { return Type{}; });
}

Results in 3 files:
__init__.pyi

from . import mo as mo
from . import mosh as mosh

mosh.pyi

class Type:
    pass

mo.pyi

def func() -> h.Type: ...

Note the h.Type on the last file, this should be _core.mosh.Type.

To fix, we check for an extra ., since we're going to be stripping it anyways.

[wjakob]

Oops, that's terrible. Thank you!

[wjakob]

Actually, should this be even more strict?

e.g.

if mod_name == self.module.__name__:
    return cls_name

instead of

if full_name.startswith(self.module.__name__ + "."):
    return full_name[len(self.module.__name__) + 1 :]

[Ahajha]

Let me try that out on my project and see if it works

[Ahajha]

Hmm, that doesn't work. I think it's because in my specific case, it's actually a nested submodule (my specific case is max._core.dialects.mo and max._core.dialects.mosh, it's trying to use .dialects as a module name in the top level __init__.pyi... Which I think should be valid? But regardless the existing change is the most correct/minimal fix of the existing options.

[wjakob]

I am not sure I follow -- the idea of this line is that we don't have to import types if they are actually from the same module. If your type is from a nested submodule, it seems to me that this simplification is not correct. Could you give a full example?

[Ahajha]

The example in the PR description shows my point. The point is that the type isn't in a nested submodule, it's in an adjacent one, but because of the names it thinks it's a nested submodule. This change is ensuring that we only actually do this for nested submodules, not the prefixing case.

[Ahajha]

Nudge @wjakob, I can add some tests here if it would help move this forward

[wjakob]

Hi Ahja,

are you sure that this fix is enough? I played around with your reproducer and seem to require the following:

diff --git a/src/stubgen.py b/src/stubgen.py
index ae7c65e..752fc67 100755
--- a/src/stubgen.py
+++ b/src/stubgen.py
@@ -683,9 +683,10 @@ class StubGen:
             if mod_name == "builtins":
                 # Simplify builtins
                 return cls_name if cls_name != "NoneType" else "None"
-            if full_name.startswith(self.module.__name__):
+
+            if mod_name == self.module.__name__:
                 # Strip away the module prefix for local classes
-                return full_name[len(self.module.__name__) + 1 :]
+                return cls_name
             elif mod_name == "typing" or mod_name == "collections.abc":
                 # Import frequently-occurring typing classes and ABCs directly
                 return self.import_object(mod_name, cls_name)
@@ -930,7 +931,7 @@ class StubGen:
             return name

         # Rewrite module name if this is relative import from a submodule
-        if module.startswith(self.module.__name__) and module != self.module.__name__:
+        if module.startswith(self.module.__name__ + '.') and module != self.module.__name__:
             module_short = module[len(self.module.__name__) :]
             if not name and as_name and module_short[0] == ".":
                 name = as_name = module_short[1:]

Could I ask you to add a test the submodule import behavior? (probably easiest to make with the existing python-based import test, this is not specific to C++ bindings).

[Ahajha]

Sounds good, I can try that patch as well. I'm going to be pretty busy this week, so might not get to it, but I'll try to next week.