search

luxonis / depthai-python

DepthAI Python Library
MIT License
336 stars 185 forks source link

Add --include-docstrings to generate_stubs.py #984

Closed filipproch closed 1 month ago

filipproch commented 4 months ago

This feature was added to stubgen in 2023 and would improve developer experience with depthai.

Bellow is quote from the original issue (on stubgen repo):

I think this would be most useful for external C or C++ modules, because some IDEs don't look for signatures or docstrings in compiled modules. Using stubgen at least fixes autocomplete for compiled modules, but if the stub files don't include docstrings, IDEs might not show any documentation when hovering over the function/class/variable in the editor (unlike with plain Python modules).

For example, Pylance in VSCode doesn't show documentation for functions in compiled modules, even with stub files generated by stubgen.

themarpe commented 4 months ago

@filipproch do you have a comparison of the output stubs?

LGTM otherwise, with an exception if mypy 1.3.0 or below has this already in? (pyproject.toml pins to that)

filipproch commented 4 months ago

@filipproch do you have a comparison of the output stubs?

LGTM otherwise, with an exception if mypy 1.3.0 or below has this already in? (pyproject.toml pins to that)

Comparison attached

Checked the compatibility (missed the version constraint before) and we would need to use mypy 1.8.0 (as 1.7 had some regressions relating to pybind11). I tested it on my machine - but if there are some specific steps/things I should check for. Otherwise I will update the PR with corrected pyproject.toml.

comparison.zip

themarpe commented 4 months ago

@filipproch please bump the mypy to 1.8.0 - there was some issues afaik on rpi's building with higher versions (missing wheels), but by the looks, versions are now in place for it.

The diff seems to expand on some parts (inner structs) - hopefully it doesn't bring in 3.8+ exclusive parts into it.

filipproch commented 4 months ago

version fixed to 1.8.0

themarpe commented 4 months ago

Kicked of a build - lets see if it goes through as expected: https://github.com/luxonis/depthai-python/actions/runs/8063852135

CC: @moratom to merge for release if all okay

moratom commented 4 months ago

Looks like the build fails for old python versions https://github.com/luxonis/depthai-python/actions/runs/8063852135/job/22026593339

Not sure if we could decouple the stubs generations from python versions? Or use different mypy versions based on the python version that the wheels is being compiled for.