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 docstrings #1614

Closed
crcrpar opened this issue Aug 11, 2020 · 14 comments · Fixed by #1627
Closed

Fix docstrings #1614

crcrpar opened this issue Aug 11, 2020 · 14 comments · Fixed by #1627
Assignees
Labels
document Documentation related. stale Exempt from stale bot labeling.

Comments

@crcrpar
Copy link
Contributor

crcrpar commented Aug 11, 2020

Sphinx>=3.1.0 doesn't build our documentation for some reason (I guess our experimental and deprecated decorators are the devils, not sure though).

At first, as #1368 said, we were optimistic about this, i.e., we thought the next stable would work. However, it's not happened yet. So it's high time we dirtied our hands to enable the latest Sphinx.

The latest ongoing pull request is #1613.

@crcrpar crcrpar added the document Documentation related. label Aug 11, 2020
@crcrpar
Copy link
Contributor Author

crcrpar commented Aug 11, 2020

Or, specify the Sphinx version, e.g., 3.0.4.

@hvy
Copy link
Member

hvy commented Aug 11, 2020

Right, good point. Let me label this issue as contribution-welcome including the investigation of the failure.

@hvy hvy added the contribution-welcome Issue that welcomes contribution. label Aug 11, 2020
@crcrpar crcrpar self-assigned this Aug 11, 2020
@harupy
Copy link
Contributor

harupy commented Aug 11, 2020

After some digging, I found that the root cause is that sphinx calls python's built-in inspect.signature with follow_wrapped=False when processing classes.

sphinx's inspect.signature which internally calls python's inspect.signature with follow_wrapped=False by default:
https://github.com/sphinx-doc/sphinx/blob/5e6da19f0e44a0ae83944fb6ce18f18f781e1a6e/sphinx/util/inspect.py#L437

ClassDocumenter calls sphinx's inspect.signature without follow_wrapped.
https://github.com/sphinx-doc/sphinx/blob/38b868cc0d0583d9a58496cd121f0bc345bf9eaa/sphinx/ext/autodoc/__init__.py#L1401

The same thing occurs in update_annotations_using_type_comments:
https://github.com/sphinx-doc/sphinx/blob/38b868cc0d0583d9a58496cd121f0bc345bf9eaa/sphinx/ext/autodoc/type_comment.py#L122

Hope this makes sense.

@harupy
Copy link
Contributor

harupy commented Aug 11, 2020

I think what we could do for now is just wait for sphinx to fix these issues.

@crcrpar
Copy link
Contributor Author

crcrpar commented Aug 11, 2020

First of all, thank you for your nice investigation!
So what do you think the best action as of now?
Do you think the current action like #1613 reasonable?

@harupy
Copy link
Contributor

harupy commented Aug 11, 2020

Yes, but I would simply specify sphinx==3.0.4 because:

  • It's frustrating to update the version specifier every time sphinx releases a new version.
  • sphinx==3.0.4 seems fine

@crcrpar
Copy link
Contributor Author

crcrpar commented Aug 12, 2020

@harupy can't agree more. Do you want to send a pull request which changes the sphinx version condition to sphinx==3.0.4?

@harupy
Copy link
Contributor

harupy commented Aug 12, 2020

Filed an issue in the sphinx repo: sphinx-doc/sphinx#8105. I'll make a PR.

@crcrpar crcrpar removed the contribution-welcome Issue that welcomes contribution. label Aug 12, 2020
@harupy
Copy link
Contributor

harupy commented Aug 12, 2020

The author of sphinx has added the issue above to the 3.3.0 milestone.

@hvy hvy closed this as completed in #1627 Aug 13, 2020
@hvy
Copy link
Member

hvy commented Aug 13, 2020

Let's keep this issue open so that we can address our TODO when 3.3.0 is released.

@hvy hvy reopened this Aug 13, 2020
@harupy
Copy link
Contributor

harupy commented Aug 13, 2020

Filed a PR in the sphinx repo: sphinx-doc/sphinx#8115 😂

@github-actions
Copy link
Contributor

This issue has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Aug 27, 2020
@crcrpar
Copy link
Contributor Author

crcrpar commented Nov 16, 2020

Sphinx 3.4.0 https://www.sphinx-doc.org/en/master/changes.html#release-3-4-0-in-development looks promising to me.

@hvy
Copy link
Member

hvy commented Dec 21, 2020

Closing as fixed by #2127. Feel free to comment or reopen if there would be any concerns.

@hvy hvy closed this as completed Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document Documentation related. stale Exempt from stale bot labeling.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants