Auto generated python stubs inconsistent with cpp API

[t-fi]

This is a stellar project, I love it! I originally came here looking for a replacement for OpenSCAD's proprietary language. And I found a little gem :)

Now, here's a technical aspect I would like to hear the authors's / maintainer's thoughts on:

Following the stub generation from bindings/python/README.md via nanobind-stubgen manifold3d I observe that the stubs don't properly reflect the API.

I do believe it is mostly a limitation of nanobind, however manifold3d can make an informed decision on whether to improve the situation.

My main issue with the stubs is that any IDE / linter / etc. will be confused, impairing the manifold3d python library user experience. For example, the Manifold cpp class has a few static constructors. Those are not marked as @staticmethod in manifold.pyi. Also, the operators for boolean combinations are not included in the the stub file (i.e. __add__ etc. are missing).

I know that there are bindings for a couple of languages, and that this is not a first class python project. Still, solid stubs could greatly improve the developer experience when using the python package.

[elalish]

I totally agree! I'm a novice Python user, so I hadn't even noticed what you mention, but it makes sense. Our Python bindings and stubs have come from various contributors who have done a lovely job getting things to their current state. If you have ideas on how to further improve, PRs are most welcome!

[elalish]

To be fair, a few things in our Python (and JS) APIs differ intentionally from our C++ API, so keep that in mind. For instance, MeshGL is mesh and Mesh isn't exposed in the bindings, plus our 2D API uses Polygons in C++, but the more powerful CrossSection directly in the bindings.

[pca006132]

This is interesting, I thought we at least have __add__! Will look into it.

[pca006132]

For the record we should probably add some sort of automated check, e.g. check if our examples type check. I think there are python type checkers out there.

[t-fi]

I am also not a python expert, especially when it comes to bindings. I have the impression that most binding generators and stub generators only work properly for a (small) subset of language constructs.

I noticed some more issues: The custom doc forwarding from manifold makes the stub generators struggle. mypy, a python typing package includes stub file generator, which crashes when applied to the manifold3d package:

 $ stubgen -p manifold3d                
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\you(ser)\PycharmProjects\manifodeling\venv\Scripts\stubgen.exe\__main__.py", line 7, in <module>
  File "mypy\stubgen.py", line 1882, in main
  File "mypy\stubgen.py", line 1711, in generate_stubs
  File "C:\Users\you(ser)\PycharmProjects\manifodeling\venv\Lib\site-packages\mypy\stubgenc.py", line 178, in generate_stub_for_c_module
    gen.generate_module()
  File "C:\Users\you(ser)\PycharmProjects\manifodeling\venv\Lib\site-packages\mypy\stubgenc.py", line 421, in generate_module
    self.generate_class_stub(name, obj, output=types)
  File "C:\Users\you(ser)\PycharmProjects\manifodeling\venv\Lib\site-packages\mypy\stubgenc.py", line 781, in generate_class_stub
    class_info = ClassInfo(class_name, "", getattr(cls, "__doc__", None), cls)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "mypy\stubutil.py", line 306, in __init__
TypeError: str or None object expected; got getset_descriptor

That problem is handled a bit more gracefully by the nanobind stub generator, in that the generated manifold.pyi file simply contains nonsense docstrings in the cases where mypy crashed. For example:

class Error(Enum):
    """
    <attribute '__doc__' of 'Error' objects>
    """

    FaceIDWrongLength: Any

    InvalidConstruction: Any

Another thing is that the packed float types like Float2x3 show up as types in signatures but are never declared. I could not find any info on which python construct corresponds to those float types. Lists and tuples seem to work though.

So there's issues on different levels. I have been digging a bit deeper, cementing my conclusion that there's no simple fix. I had a good experience with pybind11 5 years ago, not sure about the current state of the ecosystem. It might be a good alternative.

Let me create some more 3D objects with the existing bindings first, to get a feeling on how the API feels. For example, the static factory methods feel off, for me free standing functions would be more intuitive. I think the main reason for that is the pImpl pattern in the Manifold class. But again, I am still only scratching the surface :)

[pca006132]

We are open to changing the python API. Also, since we are heading towards v3.0 with a few breaking changes anyway, it is fine to have some more :)

[elalish]

FYI @wrongbad who built our Python auto-docs. It's vastly better than what we had before, but perhaps he can speak to what can still be improved and how.

[wrongbad]

My first thought is that <attribute '__doc__' of 'Error' objects> is just a sign a missing doc-string in the binding definition for Error - which could simply be added.

A side note about pybind11 is that the same original author made nanobind later and claims it is better.

[t-fi]

Interesting, I was not aware. There is a summary in the nanobind readme about the difference of scope between nanobind and pybind11:

pybind11 must deal with all of C++ to bind legacy codebases, while nanobind targets a smaller C++ subset. The codebase has to adapt to the binding tool and not the other way around

Collecting the points raised about the stubs:

• [ ] Static c++ methods are not declared static in stub
• [ ] Custom operators are missing
• [ ] Float structs like Float2x3 are referenced but not declared
• [ ] Some docs are missing

Could very well be possible to cover everything with nanobind, or by changes to the c++ API. I will look into it on the weekend.

[pca006132]

I think nanobind is not the cause of the issue here. Just an issue in the stub generator.

[elalish]

Even for v3, I'd prefer to avoid breaking changes in the C++ API, for the sake of our existing users. If we want to make the Python bindings more Pythonic and less identical to the C++ API, that feels reasonable, if it's helpful.