Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 120 commits into from
May 25, 2024

Conversation

filipproch
Copy link
Contributor

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.

Andrej Susnik and others added 13 commits June 12, 2023 17:24
```
with dai.Device() as device:
    data = device.readCcmEepromRaw(dai.CameraBoardSocket.CAM_A, 64*1024)
    np.frombuffer(bytearray(data)).tofile('ccm_eeprom.bin')
```
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
Copy link
Collaborator

@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
Copy link
Contributor Author

@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
Copy link
Collaborator

@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
Copy link
Contributor Author

version fixed to 1.8.0

@themarpe
Copy link
Collaborator

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

@themarpe themarpe requested review from moratom and removed request for themarpe February 27, 2024 11:17
@moratom
Copy link
Collaborator

moratom commented Mar 4, 2024

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.

Matevz Morato and others added 28 commits May 18, 2024 17:34
for the default case (non-burst mode). Other ToF changes:
- fix timestamps and sequence numbers
- reduce device CPU load when ToF streams, depending on FPS
- set the proper ToF exposure time in ImgFrame metadata, 796us max
- warnings for FPS capping, 80 max recommended, 88 max possible (with reduced depth quality far away)
@moratom moratom merged commit f86c134 into main May 25, 2024
37 checks passed
@moratom moratom deleted the feat/stubs-docstring-generation branch May 25, 2024 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants