search

psyinfra / onyo

text-based inventory system on top of git
ISC License
3 stars 5 forks source link

NF/DOC: generate Python API docs #611

Closed aqw closed 3 months ago

aqw commented 3 months ago

This PR is a bit bigger than I intended, but some cleanup and reorganization was necessary (and/or convenient) in order to build the docs warning free.

Major changes include:

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.76%. Comparing base (2450581) to head (87c23fb).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #611 +/- ## ========================================== - Coverage 95.76% 95.76% -0.01% ========================================== Files 69 67 -2 Lines 5621 5614 -7 Branches 1099 1099 ========================================== - Hits 5383 5376 -7 Misses 139 139 Partials 99 99 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aqw commented 3 months ago

This looks really good to me!

The only thing worth considering besides my inline comments is that typehints were applied inconsistently by different developers along files. So whenever something was not specified, they would miss in the documentation.

It might be worth to invest a few minutes into adding them where they are missing but clear, or at least running pyre -i (which should figure out some typehints automatically) and select the ones that fit the code.

Thanks for the review!

I checked through all of the methods that I modified, and confirmed that all already had sufficient type hints. We did not lose any information there.

However, I did find that the types of class attributes aren't automatically detected (which makes sense). So I restored those.

I have fixed and pushed everything. IMO, it is good to go.

There any many errors that remain in the docs, but none that are introduced by this PR. Fixing other preexisting issues will come in future PRs.