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

Fix the return documentation rendering for all model outputs #6022

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

sgugger
Copy link
Collaborator

@sgugger sgugger commented Jul 24, 2020

All PyTorch model outputs are documented from their output types. A problem is that just using the docstrings of the output class doesn't render properly on sphinx (this was also the case before the new model outputs were introduced).

This PR adds a function that converts the args part of the docstrings of the output class to render properly on our doc. You can see the transformation by looking here for the docs before this PR and here for after it's merged (it's one example, but it will give the same result for all models). Scroll a bit to get to the return part of the doc.

@codecov
Copy link

codecov bot commented Jul 24, 2020

Codecov Report

Merging #6022 into master will increase coverage by 0.40%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6022      +/-   ##
==========================================
+ Coverage   78.29%   78.70%   +0.40%     
==========================================
  Files         146      146              
  Lines       26249    26268      +19     
==========================================
+ Hits        20552    20674     +122     
+ Misses       5697     5594     -103     
Impacted Files Coverage Δ
src/transformers/modeling_transfo_xl.py 79.12% <ø> (ø)
src/transformers/file_utils.py 82.20% <100.00%> (+1.00%) ⬆️
src/transformers/modeling_tf_roberta.py 43.98% <0.00%> (-49.38%) ⬇️
src/transformers/generation_tf_utils.py 86.21% <0.00%> (ø)
src/transformers/modeling_openai.py 82.25% <0.00%> (+1.29%) ⬆️
src/transformers/modeling_tf_openai.py 95.18% <0.00%> (+74.91%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3996041...345372d. Read the comment docs.

blocks = []
current_block = ""
for line in output_args_doc.split("\n"):
# If the indent is the same as the beginning, the line is the name of new arg.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is crazy!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you call _convert_output_args_doc anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a little bit under in _prepare_output_docstrings

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of find the old one more readable, but I agree it's better to be more coherent between inputs/outputs. Thanks for looking into it!

@sgugger sgugger merged commit 1246b20 into master Jul 27, 2020
@sgugger sgugger deleted the fix_return_doc branch July 27, 2020 13:19
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.

None yet

3 participants