quantumlib / Cirq

A Python framework for creating, editing, and invoking Noisy Intermediate Scale Quantum (NISQ) circuits.
Apache License 2.0
4.23k stars 1.01k forks source link

Improve API reference #3667

Open dkafri opened 3 years ago

dkafri commented 3 years ago

Is your feature request related to a use case or problem? Please describe. I would like to look at the API for the Moment class. I spent a while sifting through the new Cirq documentation but I could not find it. I tried the search bar but it was not there. I tried to look at the code on github but a simple search does not reveal where the Moment class is defined. My last resort was to use my IDE (PyCharm) to find its definition, but some users do not have this capability.

Describe the solution you'd like Update the documentation page to make it easy to search for class definitions.

[optional] Describe alternatives/workarounds you've considered See above.

[optional] Additional context (e.g. screenshots)

What is the urgency from your perspective for this issue? Is it blocking important work? P1 - I need this no later than the next release (end of quarter)

rmlarose commented 3 years ago

cirq.Moment appears in the API reference for me:

image

as well as by searching "Moment" on quantumai.google:

image

maffoo commented 3 years ago

It is a bit unfortunate that the "all symbols" page shows all of cirq.Moment, cirq.ops.Moment, and cirq.ops.moment.Moment. All of those link to a page showing cirq.ops.Moment with the other two (cirq.Moment and cirq.ops.moment.Moment) listed as aliases, even though cirq.Moment is the preferred way for users to access it.

dkafri commented 3 years ago

I guess I didn't think to scroll through that whole list at the front page. When I searched "Moment" in the search bar I did not see the link to the API reference. screenshot-2021-01-13-09-15-04

95-martin-orion commented 3 years ago

Seems like the issue here is the discrepancy between the instant drop-down list and the search results, as I can reproduce both experiences described above. Oddly, the search seems to return just about every page except the Moment page - even other files under .../reference/python/cirq. Is the ops directory somehow hidden from the search results?

rmlarose commented 3 years ago

Indeed there's a discrepancy between suggested searches and search results. I don't think ops is hidden however as it shows up with a search for ops, albeit below several entries for the openfermion ops module.

image

balopat commented 3 years ago

Hi All, unfortunately we don't have too much control over this experience :(. Our users will have to get used to the dropdown behavior. Hitting enter takes you to a Google search of the site that is hard to influence, as it's based on the indexing of Search. On top of that this is not something that we can influence from this repo at all, but the internal devsite people might have some wisdom around it. I can share the internal bugtracking component with you, @dkafri offline. As we can't really do anything about it here, I would close this issue.

balopat commented 3 years ago

Actually there is one thing that we can do: link to the mentions of the Moment class to the reference. Which is as simple as rewriting Moment to cirq.Moment everywhere. This will automatically get converted to a link pointing to the API docs. That way at least, if the Google search takes you to a page, you can still click into the link.

95-martin-orion commented 3 years ago

Actually there is one thing that we can do: link to the mentions of the Moment class to the reference. Which is as simple as rewriting Moment to cirq.Moment everywhere.

Of course, since this presumably affects all objects (not just Moment), the required change is much larger than this. Still simple, but likely to touch a lot more of the Cirq codebase.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

unaiic commented 3 years ago

@balopat @95-martin-orion Do you mean rewriting all mentions to objects in the docs to cirq.ObjectName?

95-martin-orion commented 3 years ago

@balopat @95-martin-orion Do you mean rewriting all mentions to objects in the docs to cirq.ObjectName?

That's roughly what we're suggesting, but if you'd like to take this on I recommend making an example PR with a single file before changing all docs. We'll need to balance this issue with overall readability of the docs.

Note that objects referenced this way must appear in the top-level cirq/__init__.py file to be auto-linked.

unaiic commented 3 years ago

@95-martin-orion Fine, I'll work on this. Thank you for the notes.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

95-martin-orion commented 3 years ago

Switched won't-fix for accepted since a solution is in progress.

Strilanc commented 3 years ago

Our users will have to get used to the dropdown behavior.

It seems like a contradiction in terms for users to have to "get used to" the behavior of documentation.