Add Python stubs for rosbag2_py (#1459)

[r7vme]

Fixes #1459 (related PR #1463)

[r7vme]

@MichaelOrlov this is initial PR not for review yet.

.pyi files are autogenerated by pybind11-stubgen manually and I just merged generated files to git.

I see two options:

1. (preferred) Create a script to autogenerate those files during the build. But for this we will need to add pip dependency pybind11-stubgen==2.4.2. Where is the right place to do it?
2. Merge manually generted files and expect developers regenerte them manually

[clalancette]

1. (preferred) Create a script to autogenerate those files during the build. But for this we will need to add pip dependency pybind11-stubgen==2.4.2. Where is the right place to do it?

This will only work if there is an Ubuntu .deb and RHEL .rpm package that contains stubgen. Without that, there is no way we could actually release this on the buildfarm.

[r7vme]

@clalancette unfortunately there is no pybind11-stubgen in Ubuntu or RHEL repos.

After reading pybind/pybind11#2350 , seems like stubgen from mypy may be a preferred choice over pybind11-stubgen. It also available in apt repos. Unfortunately stubgen -p rosbag2_py requires module to be installed. I was not able to run stubgen w/o compiling module first.

For now I pushed manually generated files from mypy's stubgen.

[MichaelOrlov]

@christophebedard Your help will be appreciated with figuring out if can create a script for github actions to autogenerate .pyi stub files from mypy's stubgen?

[christophebedard]

Having CI generate files and commit them is possible, but probably annoying. The simpler solution (and the one I would recommend) is to set up CI to generate the files and then do git diff --exit-code, which will make the CI job fail if the files need to be re-generated. So you don't have to deal with setting up CI to commit the files, but CI will tell you if you need to re-generate files.

For example: https://github.com/ros-tooling/action-ros-ci/blob/634e9ac5fb970763bd6207e31e1875d0081be71e/.github/workflows/pr.yml#L19

[r7vme]

@christophebedard thanks, I added following to .github/workflows/test.yml.

    - name: Is regeneration of Python stubs required?
      run: |
        sudo apt update && sudo apt -y install mypy=0.942-1ubuntu1
        source install/setup.sh
        stubgen -p rosbag2_py -o src/ros2/rosbag2/rosbag2_py/rosbag2_py/stubs
        git --git-dir src/ros2/rosbag2/.git --work-tree src/ros2/rosbag2 diff --exit-code
      working-directory: ${{ steps.action-ros-ci.outputs.ros-workspace-directory-name }}
      shell: bash

However, it fails because seems src/ros2/rosbag2 is not a git repo. Do you have any suggestions? See here

[r7vme]

@MichaelOrlov @christophebedard could you please take a look?

[christophebedard]

The CI code looks good to me.

You could try to make a small manual change to one of the stubs files and push it to validate that the CI job catches it and fails, but that might be overkill given that it did "properly" fail before due to a different mypy version.

[MichaelOrlov]

@r7vme BTW. I've checked how hints work in CLion without .pyi files.
They work out of the box and are surprisingly fast and accurate!

This is one more point to start using CLion 😉
The CLion is smart enough and creating similar stubs during the project parsing

class StorageOptions(__pybind11_builtins.pybind11_object):
    # no doc
    def __init__(self, uri, storage_id='', max_bagfile_size=0, max_bagfile_duration=0, max_cache_size=0, storage_preset_profile='', storage_config_uri='', snapshot_mode=False, custom_data, p_str=None, p_str=None_1, *args, **kwargs): # real signature unknown; NOTE: unreliably restored from __doc__ 
        """ __init__(self: rosbag2_py._storage.StorageOptions, uri: str, storage_id: str = '', max_bagfile_size: int = 0, max_bagfile_duration: int = 0, max_cache_size: int = 0, storage_preset_profile: str = '', storage_config_uri: str = '', snapshot_mode: bool = False, custom_data: Dict[str, str] = {}) -> None """
        pass

    custom_data = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    max_bagfile_duration = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    max_bagfile_size = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    max_cache_size = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    snapshot_mode = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    storage_config_uri = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    storage_id = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    storage_preset_profile = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

    uri = property(lambda self: object(), lambda self, v: None, lambda self: None)  # default

[MichaelOrlov]

@r7vme @christophebedard I came across this discussion pybind/pybind11#2350
and also someone's Python script for generating .pyi files
https://github.com/robotpy/robotpy-build/blob/dee9719fb066791b21c4a2b38b7fc2094f37b8c0/robotpy_build/command/build_pyi.py#L119-L140
Curious about if it would be useful for us?

[r7vme]

@MichaelOrlov

The CLion is smart enough and creating similar stubs during the project parsing

cool, seems it is worth the price.

@r7vme @christophebedard I came across this discussion pybind/pybind11#2350
and also someone's Python script for generating .pyi files
https://github.com/robotpy/robotpy-build/blob/dee9719fb066791b21c4a2b38b7fc2094f37b8c0/robotpy_build/command/build_pyi.py#L119-L140
Curious about if it would be useful for us?

I also saw this script. The problem that setup.py generated automatically by ament and I did not find an easy way to call build_pyi.py during that process. Also I was not able to find a way to call stubgen (mypy) before installing the module (i.e. module have to be installed to in order stubgen to work).

[r7vme]

@christophebedard

You could try to make a small manual change to one of the stubs files and push it to validate that the CI job catches it and fails, but that might be overkill given that it did "properly" fail before due to a different mypy version.

Validated. CI will fail if there are changes in pyi files (link)

[r7vme]

@MichaelOrlov ready for another look

[MichaelOrlov]

@r7vme As regards:

I also saw this script. The problem that setup.py generated automatically by ament and I did not find an easy way to call build_pyi.py during that process. Also I was not able to find a way to call stubgen (mypy) before installing the module (i.e. module have to be installed to in order stubgen to work).

Sorry, I can't help here either. Maybe @christophebedard or @emersonknapp will have some ideas.
I was thinking that it would be the ideal solution if stub files were generated during the package build time and we would add them to the .gitignore or if they would be in the build subfolder and IDE would still see them. In this case, we will not need to add stub files to the repo and check them with every commit.

[r7vme]

I was thinking that it would be the ideal solution if stub files were generated during the package build time

@MichaelOrlov There is one problem. stubgen works with compiled python module (i.e. requires python -c "import rosbag2_py" to work). Before Python module is built (pybind's .so libraries) and installed we can not import this module.

[MichaelOrlov]

@r7vme

I was thinking that it would be the ideal solution if stub files were generated during the package build time

@MichaelOrlov There is one problem. stubgen works with compiled python module (i.e. requires python -c "import rosbag2_py" to work). Before Python module is built (pybind's .so libraries) and installed we can not import this module.

I found a good idea on how to solve this problem here https://stackoverflow.com/questions/65876895/how-to-structure-and-distribute-pybind11-extension-with-stubs
i.e. Alternatively, distribute .pyi files separately as a standalone package (e.g. rosbag2_py_stubs).

In this case, we can declare rosbag2_py package as a dependency for the rosbag2_py_stubs and it will be compiled and installed before trying to build rosbag2_py_stubs. Inside rosbag2_py_stubs will be a Python script for generating .pyi files and perhaps we could copy them somewhere in the install folder from the CMake script.

@r7vme What do you think about this idea?

[r7vme]

@MichaelOrlov I see one main issue, users have to install rosbag2_py-stubs manually. I suspect that average user won't get benefits in that case.

[MichaelOrlov]

@MichaelOrlov I see one main issue, users have to install rosbag2_py-stubs manually. I suspect that average user won't get benefits in that case.

Well, if it will be a separate package and pyi stubs will be as build artefacts in the install folder - they will be as part of the release and if user will be using ROS 2 installation from deb binaries will not need to manually install rosbag2_py-stubs.

The only case when need to manually rebuild and install rosbag2_py-stubs package is when the user needs to rebuild rosbag2 from sources or when doing changes in the pybynd11 interfaces.
But that is expected anyway even in the current approach.

In cases when need to build rosbag2 from sources we recommend to use a meta package rosbag2 for the build command to make sure that everything belonging to the rosbag2 will be built. i.e. run command colocn build --packages-up-to rosbag2.
We usually adding all other packages as dependencies to the rosbag2 package.

[r7vme]

@MichaelOrlov I agree that having fully-automated solution is better.

However, before investing time into doing this in a separate package, I want to point out:

1. PEP 561 recommends to ship .pyi alongside .py files. Separate stub package is more a workaround for e.g. third party libraries.
2. We should also consider realistic man-hours that contributors have :) full automation probably be more development and maintenance work (as we adding another ROS package and more CI scripting).

For package maintainers wishing to ship stub files containing all of their type information, it is preferred that the *.pyi stubs are alongside the corresponding *.py files.

I personally prefer to keep current semi-automated solution, if smth won't work we can always invest more time and automate it fully. Also to fully satisfy original issue #1459, we have to make another PR that improves pybind type information (example #1459 (comment))

[MichaelOrlov]

@r7vme Tnanks for fixes during the review. LGTM!

[MichaelOrlov]

Gist: https://gist.githubusercontent.com/MichaelOrlov/1de9b06fce7ec3ae4981f6bb56c1cad9/raw/dc192ddb97855f6ce0b3f43e057d61e94de66041/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_py rosbag2_tests
TEST args: --packages-above rosbag2_py rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13485

• Linux
• Linux-aarch64
• Windows

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-for-2024-03-21/36814/1