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

Make sphinx not generate bogus undocumented return type in autoclass #3974

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

ktbarrett
Copy link
Member

This should remove the Return type: statement from the description of Join. Apparently Sphinx thinks class definitions have return types that aren't the class type and it's losing its mind attempting to deduce it so we just turn that logic off and do it manually.

@ktbarrett ktbarrett requested a review from a team June 25, 2024 03:33
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.63%. Comparing base (2bfb730) to head (b2d97a8).
Report is 199 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3974      +/-   ##
==========================================
+ Coverage   72.19%   72.63%   +0.43%     
==========================================
  Files          50       50              
  Lines        8028     8025       -3     
  Branches     2223     2211      -12     
==========================================
+ Hits         5796     5829      +33     
+ Misses       1730     1682      -48     
- Partials      502      514      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marlonjames
Copy link
Contributor

Return type: goes from 77 hits to 16 on the library reference docs page.
In general I like the auto-generated return type info, so maybe as suggested in the sphinx issue we do whatever to override the Join autoclass directive causing the issue?

@ktbarrett
Copy link
Member Author

ktbarrett commented Jun 25, 2024

@marlonjames From the issue:

I'd say it's a bug but I don't really have time (nor the mood) to fix this one.

It seems the return type is included when the return is documented in this mode. When we enable docstring checking in ruff, one of the checks includes enforcing the use of Args, Returns, etc. So we can compensate manually by adding documentation for Returns (which we need to do anyways) and won't have to worry about missing any since they are checked.

@ktbarrett
Copy link
Member Author

Seems like there's no feature for turning off the return type deduction either.

@marlonjames marlonjames added the category:docs documentation issues and fixes label Jul 12, 2024
@marlonjames
Copy link
Contributor

Looking at the docs again, if we include
:rtype: type
in the docstring we should be able to add back manually the ones we want.
Too bad we can't disable it just for classes where it fails.

@ktbarrett ktbarrett merged commit af30a2f into cocotb:master Aug 1, 2024
29 checks passed
@ktbarrett ktbarrett deleted the fix-docs branch August 1, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:docs documentation issues and fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants