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

Associate docstrings with public constants #2471

Merged
merged 7 commits into from
Nov 4, 2019
Merged

Associate docstrings with public constants #2471

merged 7 commits into from
Nov 4, 2019

Conversation

Strilanc
Copy link
Contributor

@Strilanc Strilanc commented Nov 1, 2019

- Add `cirq/_doc.py` with a `document` function
- Pass public constants into `document` when they are initialized
- Fixes the fact that e.g. [`cirq.OP_TREE` was being documented using `typing.Union`'s docstring](https://cirq.readthedocs.io/en/stable/generated/cirq.OP_TREE.html
@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Nov 1, 2019
@mpharrigan
Copy link
Collaborator

So there's a way to do this in sphinx -- sortof.

If you read the documentation (particularly for the autodata directive) it says you can just add a docstring under it

OP_TREE = Union[Operation, OpTree]
"""loo loo loo"""

At first blush this doesn't work. That's because api.rst lists cirq.OP_TREE. After some digging, these data docstrings don't make it through import forwarding (sphinx-doc/sphinx#6495). So if you change the listing to ~cirq.ops.op_tree.OP_TREE it shows the docstring. The ~ means "hide the module qualifiers" so it still shows up in the API table as just OP_TREE; but the title of the page and its toc entry has the fully qualified name (which is bad). One could probably fix this with custom autodoc templates. It gets dicey for the places where we do want some sort of name qualification, like in google.XXX and testing.XXX

I think it's important to clearly understand the limitations of autodoc before we re-implement (parts of) it

@Strilanc
Copy link
Contributor Author

Strilanc commented Nov 1, 2019

Where did you find documentation on the tilde modifier?

@mpharrigan
Copy link
Collaborator

A very good question. I learned it at some point and was actually trying to find documentation on it to see if there were additional options to control the degree of name qualification but alas I couldn't find anything about ~. Empirically it works though

@Strilanc
Copy link
Contributor Author

Strilanc commented Nov 1, 2019

After group discussion (including @mpharrigan ), we decided to go with this document function. At least until sphynx finishes their "some future version" milestone fixing it.

@Strilanc Strilanc added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 4, 2019
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Nov 4, 2019
@CirqBot CirqBot merged commit 9c0eb4c into master Nov 4, 2019
@CirqBot CirqBot deleted the doczar1 branch November 4, 2019 22:01
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants