coin3d / pivy

python bindings to coin3d
ISC License
53 stars 37 forks source link

Typings for mypy #71

Open Vanuan opened 3 years ago

Vanuan commented 3 years ago

Is there any chance pivy typings could be provided without too much effort? Or maybe such a typing package already exists?

E.g. like this

class SbVec3f:
    def __init__(self): ...
    def getValue(self) -> Tuple[float, float, float]: ...

class SoSFVec3f:
    def __init__(self): ...
    def getValue(self) -> SbVec3f: ...
    def setValue(self, value: Sequence[float]) -> None: ...

class SbRotation:
    def __init__(self): ...

class SoSFRotation:
    def __init__(self): ...
    def setValue(self, value: List[float]) -> None: ...

class SoCamera:
    position: SoSFVec3f
    orientation: SoSFRotation

    def __init__(self): ...
looooo commented 3 years ago

I am not quite sure what you want. Pivy is generated with swig. So maybe you should look into swig-docs to see if there is such a feature.

Vanuan commented 3 years ago

So, there's this provisional standard in Python 3.5, PEP-484 Type Hints.

It's based on PEP 3107 -- Function Annotations syntax (Python 3.0) and allows you to define types for static analysis, so that type checking can be used to catch bugs at build-time in IDE and using a mypy standalone utility.

It's especially suitable for C++ python bindings, since C++ is inherently strictly typed language.

To define types for bindings, you provide a stub file, with *.pyi extension.

So, what I want is this set of *.pyi files to be provided with pivy.coin pip package.


It looks like it is supported via autodoc: http://swig.org/Doc3.0/Python.html#Python_nn67

But it's not quite the same. Somebody has already requested this feature in the swig repo on GH: https://github.com/swig/swig/issues/735


Would you be willing to provide stub files in the pip package if I were to code them manually?

looooo commented 3 years ago

the pip packages are not updated. we use conda, and other system package-manager. There is the possibility to create wheels from conda-packages, but I am not sure this is the best way to proceed. It would be best if we can automate the process. actually the py3 option for swig adds the types to the function definitions. not sure if this is related. Best to start by building pivy yourself, if you haven't done it already.

Vanuan commented 3 years ago

Well, by "pip package" I meant "to be installable using setup.py install"

looooo commented 3 years ago

we recently updated to cmake. so it would be best if you will use this configuration for any additional features.

Vanuan commented 3 years ago

So it doesn't use distutils anymore? Could you please update README? https://github.com/coin3d/pivy/blob/master/README.md

looooo commented 3 years ago

It should work with both (cmake or distutils). But I guess cmake will be the future as it is consistent with the rest of the coin-tools. I added some instructions: https://github.com/coin3d/pivy/commit/c7854b7b3c16dbec152d80578d5e874edaf18879

looooo commented 3 years ago

found this here: https://github.com/tpaviot/pythonocc-core/blob/master/src/SWIG_files/wrapper/AIS.pyi

pythonocc-core uses also swig. So it might be interesting to know how these files are created.

edit: some more insights https://github.com/tpaviot/pythonocc-core/issues/789

Vanuan commented 3 years ago

As far as I understood: pythonocc generates python binding using a custom pythonocc-generator by parsing C++ header files and creating SWIG *.i files.

In the following PR this pythonocc-generator was enriched with a capability to generate .pyi files in addition to SWIG .i files: https://github.com/tpaviot/pythonocc-generator/pull/79

In pivy, as far as I see, SWIG files are not generated but are rather coded manually.

So no, pythonocc approach is not applicable without heavy reengineering.


Another approach is using -py3 swig flag to generate proxy classes with function annotations. But unfortunately, those use C++ types. Also, as I see pivy doesn't use swig proxy classes.

I see there was an attempt to do so here: https://gist.github.com/ceremcem/04f19e862230c342c2622bb1f5f6f04b


Manual coding. This is the most robust but the most time consuming approach.


Stubgen

This looks promising: https://mypy.readthedocs.io/en/stable/stubgen.html

But of course it is just a draft for manually edited files. It lacks proper types and has no documentation.


MonkeyType

https://github.com/Instagram/MonkeyType

This is the most advanced generator that infers types. But it requires runtime usage. That is the code is actually run, not statically analysed. So you need a good test coverage.


Out of those, I would go for a combination of manual coding with some generator which would parse out the Doxygen documentation

looooo commented 3 years ago

thanks for the research. Monkeytype sounds like a very nice way to 1. increase test coverage and 2. get typings. Currently there are not many tests. But I will try monkeytype in the next days.

Vanuan commented 3 years ago

I've tried the CppHeaderParser. It works, but unfortunately, API documentation is located at the *.cpp files rather than in header files and CppHeaderParser can't parse out doxygen comments out of method implementation. The practice of putting API documentation to cpp files rather than header files appears to be for the sake of avoiding rebuilds when updating documentation. Yeap, C++ sucks.

It appears the only way is to use Doxygen's XML output.

MonkeyType wouldn't work well, as python docstrings currently only contain type annotations, not the C++ descriptions.

Vanuan commented 3 years ago

Here's a CppHeaderParser approach. Requires pip install robotpy-cppheaderparser

  1. Configure coin3d to genarate coin_build: cmake -Hcoin -Bcoin_build -G "Unix Makefiles" -DCMAKE_INSTALL_PREFIX=/usr/local
  2. Preprocess C++ header file using one of two approaches (pip install pcpp vs gcc):
    • pcpp -I coin/include/ -I /usr/lib/gcc/x86_64-linux-gnu/8/include -I /usr/lib/gcc/x86_64-linux-gnu/8/include-fixed -I /usr/include/x86_64-linux-gnu/ -I /usr/local/include/x86_64-linux-gnu -I /usr/local/include/ -I /usr/include -I coin_build/include/ coin/include/Inventor/nodes/SoNodes.h > coin_SoNode.h
    • gcc -E -I coin/include/-I coin_build/include/ coin/include/Inventor/nodes/SoNodes.h > coin_SoNode.h
  3. generate_stubs.py: https://gist.github.com/Vanuan/85fd55decd16e6adc352d3ec4422e950
Vanuan commented 3 years ago

The main issue is that C++ types don't quite map to Python types. Python lacks the assignment operator override. Also, some setValues are mapped manually.

It means there are a lot of errors like this:

src/Mod/Arch/ArchPanel.py:1458: error: Incompatible types in assignment (expression has type "int", variable has type "SoSFInt32")
src/Mod/Arch/ArchSectionPlane.py:739: error: Incompatible types in assignment (expression has type "bool", variable has type "SoSFBool")
src/Mod/Arch/ArchSectionPlane.py:755: error: No overload variant of "setValues" matches argument type "List[Tuple[float, float, float]]"

Have no idea how to fix those so far.

The SWIG interface files (*.i) do not provide any declarative mechanism for that.

looooo commented 3 years ago

So what is the conclusion? manually editing the pyi-files?

Vanuan commented 3 years ago

Maybe some hybrid solution. For example, some manual mapping:

SbVec3f = Tuple[float, float, float]
SoMFVec3f = Sequence[SbVec3f]

But I don't have enough experience with Coin to determine the right mappings.

Also the issue is that even if you do it manually, you can't quite mimic automatic type conversion that is implemented in C++.

For example, this:

condition: SoSFBool = True

can only be implemented like this:

condition: SoSFBool = SoSFBool(True)

If you rather specify SoSFBool to be an alias of bool, you would have another issue:

condition: SoSFBool = True
condition.getValue() # bool doesn't have an attribute 'getValue'

Still need to investigate how it works in runtime.

Vanuan commented 3 years ago

Figured out the SoMF*::setValues pattern: image

>>> coin._coin.SoMFVec3f_setValues([(1,1,1)])
Traceback (most recent call last):
  File "<input>", line 1, in <module>
TypeError: Wrong number or type of arguments for overloaded function 'SoMFVec3f_setValues'.
  Possible C/C++ prototypes are:
    SoMFVec3f::setValues(int const,int const,SbVec3f const *)
    SoMFVec3f::setValues(int,int,float const [][3])

It means the following proxy method prototypes are defined in SoMFVec3f:

# original C++ prototypes
@overload
def setValues(self, start: int, num: int, newvals: Sequence[SbVec3f]): ...
@overload
def setValues(self, start: int, num: int, newvals: Sequence[Tuple[float, float, float]]): ...

# proxy prototypes with 2 arguments
@overload
def setValues(self, Sequence[SbVec3f]): ...
@overload
def setValues(self, Sequence[Tuple[float, float, float]]]): ...

# proxy prototypes with 3 arguments
@overload
def setValues(self, start: int, Sequence[SbVec3f]): ...
@overload
def setValues(self, start: int, Sequence[Tuple[float, float, float]]]): ...
Vanuan commented 3 years ago

Here's what I have so far: https://github.com/Vanuan/pivy_coin_stub_generator

TODO:

looooo commented 3 years ago

any progress on this @Vanuan? can we already include this in the packages?

Vanuan commented 3 years ago

@looooo Coin is a massive library. I couldn't make much progress on generating typings. Only stubs so far, which should be edited manually to match C++ types to corresponding Python types.

I appreciate your interest and everything you do to maintain Coin/Pivy.

Currently, my hope is that there's more interest in Coin. To do that, I've tried to update the README page: https://github.com/coin3d/coin/pull/439 But that was quite an effort. I think there should be more quickstart guides and examples to make coin more appealing for new users not related to FreeCAD.

You're doing a tremendous work by maintaining this library. Keep it up.

Vanuan commented 3 years ago

My latest progress is here: https://github.com/Vanuan/freecad-python-stubs/tree/in_progress/stubs/pivy/coin

looooo commented 3 years ago

@Vanuan thanks for your work on this issue. Whenever you think we should integrate something into pivy / pivy-packages, please ping me.