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

Epic: link to source code / GitHub in API docs #454

Closed
aeddins-ibm opened this issue Nov 30, 2023 · 9 comments
Closed

Epic: link to source code / GitHub in API docs #454

aeddins-ibm opened this issue Nov 30, 2023 · 9 comments
Assignees
Labels
API docs epic 🏔️ infra 🏗️ user feedback 📥 General user feedback for design research purposes

Comments

@aeddins-ibm
Copy link

aeddins-ibm commented Nov 30, 2023

Feedback

Before the API docs migrated to ibm.com, IIRC the doc pages often included links to the relevant Github source code. [edit: apparently it may not have been github, but some other hosted version of the source code?] These are useful for verifying details of how the code behaves. It would be nice to have those back if it's not too technically difficult.

An example workflow:
A user needs to know something about something in the API, say SparsePauliOp. They google "SparsePauliOp" and get the doc page (usually the top result), which might already have the info they need. If it doesn't, then the Github link on that page means with 1 click they get to the relevant source code, where they can find out what they need to know.

Now with the new docs, if the doc page does not have the desired info, then the user will probably do a new Google search "github qiskit", select the second result for qiskit/qiskit, then in that page use the Github search box to search for "SparsePauliOp", pick the most promising search-result snippet, etc. Of course it's doable but it adds time/clicks to something a user/developer might do many times, so direct links from the docs would be nice to have.

Other comments

No response

@Eric-Arellano
Copy link
Collaborator

Thanks for opening this and upvoting, everyone! This has been by far our most requested feature request so far. I've marked it priority 0 for our next milestone, Qiskit 1.0 around February 13.

--

I think it will make the most sense for this to be a link that takes you back to GitHub, rather than how qiskit.org showed the actual source code directly. We had gotten several requests for qiskit.org that people would rather have the source link, including because it's nice to have things like changing the code branch & navigating the rest of the repo.

Beyond what is more desirable, I suspect it will be easier to implement a URL to GitHub than rendering the source code directly in ibm.com. Less to style, less data to store, etc

If we do go with storing links, that means we need to update the Sphinx config in Qiskit/qiskit to compute the correct URL, which is discussed at Qiskit/qiskit_sphinx_theme#186 (comment) how we'd do it. Then, once Qiskit/qiskit API docs are generating the proper URLs, we'd need to update this repository's script to convert those API docs into Markdown to preserve the URLs and render them in a way that's useful for the ibm.com website, such as if we need to add a CSS class name to the link for the styling to work well.

@javabster
Copy link
Collaborator

Yeah I agree I think a GitHub link would be preferred, I'm guessing our link checker would be able to catch if any of the links get out of sync? I'm imagining a case where a branch name changes (e.g when it goes from main to stable to a historical version number) or something and then the url no longer works

@Eric-Arellano
Copy link
Collaborator

I'm guessing our link checker would be able to catch if any of the links get out of sync?

Yes, although only as a nightly cron job because external link checking was really slow and sometimes flaky.

I'm imagining a case where a branch name changes (e.g when it goes from main to stable to a historical version number) or something and then the url no longer works

The link would always be a permalink to the exact commit used when the docs were built. That's important because code can change between versions.

@Eric-Arellano
Copy link
Collaborator

Thanks again for the upvotes everyone!

FYI I split this into three tasks so that we can more easily track how things are going:

The designers are going to start soon on figuring out a design. Let us know if anyone wants to try out #517! Otherwise, one of us core maintainers will try to get to it near the end of December or earlier January.

@javabster
Copy link
Collaborator

question - is the plan for this source code link to just be available for the api-ref pages? If possible I think it could be a nice addition to the non-api-ref pages as well, what do you think?

@Eric-Arellano
Copy link
Collaborator

question - is the plan for this source code link to just be available for the api-ref pages?

Yes, this is a link to the original source code for API files.

If possible I think it could be a nice addition to the non-api-ref pages as well, what do you think?

I agree, but this should be tracked by a separate issue/epic. The visual design and the implementation look quite different. With API docs, the design is inline with the content, and we'll use Sphinx to determine what the exact URL will be.

Eric-Arellano added a commit that referenced this issue Jan 11, 2024
Implements #454 for
projects that are using `sphinx.ext.viewcode`. All 3 of our projects
were historically using it until I turned it off after removing
qiskit.org, so the historical API docs are good to go. I'll restore it
for current versions of these projects and regenerate the docs.

This implementation is not a perfect implementation:

1. The visual design is a little awkward, especially the lack of
padding. This can be improved via
#518

<img width="571" alt="Screenshot 2024-01-11 at 12 28 07 PM"
src="https://github.com/Qiskit/documentation/assets/14852634/2d915b94-ec48-445a-a174-ace628655b4b">

2. The link only takes you to the overall code page, not the specific
lines. This could be improved via
#517.

But it's good enough to not block on these improvements.

This PR regenerates all Runtime historical versions, but not any current
versions nor Qiskit historical versions.

## How source code URLs are determined

`sphinx.ext.viewcode` embeds a copy of every Python file used in API
docs and uses internal relative links like
`../modules/qiskit_ibm_runtime/ibm_backend.html`. They correspond to
Python files we can be confident exist. We transform those relative
links into GitHub links here:


https://github.com/Qiskit/documentation/blob/790e9372f64ab7d5f15eaccc229b4d0765781d44/scripts/lib/api/processHtml.ts#L94-L105


https://github.com/Qiskit/documentation/blob/790e9372f64ab7d5f15eaccc229b4d0765781d44/scripts/lib/api/processHtml.ts#L163-L183

Our links assume that there is a branch called
`stable/<versionWithoutPatch` in GitHub, like `stable/0.8`.

## Replacing `[source]` with `GitHub ↗︎`

We remove the original link from Sphinx and replace it with our own.
This is important so that the link is not included in the `<code>` HTML
element incorrectly. It also allows us to set a custom link label and
`title` (the text when highlighting).
@Eric-Arellano
Copy link
Collaborator

Great news! We got this working in #620 today and it's now live for historical API docs for Runtime 🎉

Screenshot 2024-01-11 at 4 42 30 PM

Provider will be added in #624. The current version of Runtime will be added once Qiskit/qiskit-ibm-runtime#1312 is merged. For Qiskit, we'll regenerate all the historical API versions and current docs once Qiskit/qiskit#11548 is merged.

--

This initial implementation isn't perfect. #517 would make the links be more precise so it takes you to the exact lines, rather than the overall file; contributions appreciated on that! I think it should be doable for someone new to the project.

#518 tracks making the visual design a little less awkward with how there isn't padding. But we didn't want to block this functionality on that once we realized yesterday this easier approach was possible.

--

Thank you everyone for the feedback on voting on this! It was really helpful for prioritization.

You may also want to subscribe to #479 for overall improvements to the API docs, one of the team's highest priorities.

@Eric-Arellano Eric-Arellano moved this to In Progress in Docs Planning Jan 16, 2024
@Eric-Arellano
Copy link
Collaborator

Eric-Arellano commented Jan 18, 2024

Update: Provider and all of Runtime are live as of yesterday 🎉

We've been working the past few days to get Qiskit source code links live. We've hit two road blocks but are making progress on fixing them:

  1. For Qiskit 0.44 and earlier, the source code files refer to multiple GitHub repositories because Qiskit was a "metapackage" comprised of several smaller projects. Edit: fixed by Handle historical Qiskit versions with GitHub source links #647
  2. For Qiskit 0.45, the source code links work. But a recent change to how we render math expressions is blocking us from deploying those docs :/

@Eric-Arellano Eric-Arellano self-assigned this Jan 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 19, 2024
Part of #454. Historical
versions of Qiskit docs are tricky because of qiskit-metapackage
referring to multiple distinct GitHub repositories.
github-merge-queue bot pushed a commit that referenced this issue Jan 19, 2024
Part of #454. Note that
Qiskit 0.45 refers to Qiskit Terra 0.25.
Eric-Arellano pushed a commit that referenced this issue Jan 19, 2024
Part of #454. Note that
Qiskit 0.43 refers to Qiskit Terra 0.24.
@Eric-Arellano
Copy link
Collaborator

This is now live for every version of the API docs 🚀 We used our link checker to validate that all of the URLs to GitHub work.

See the comment above #454 (comment) for possible follow up improvements.

Thank you everyone for voting on this issue!

@github-project-automation github-project-automation bot moved this from In Progress to Done in Docs Planning Jan 24, 2024
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Implements Qiskit#454 for
projects that are using `sphinx.ext.viewcode`. All 3 of our projects
were historically using it until I turned it off after removing
qiskit.org, so the historical API docs are good to go. I'll restore it
for current versions of these projects and regenerate the docs.

This implementation is not a perfect implementation:

1. The visual design is a little awkward, especially the lack of
padding. This can be improved via
Qiskit#518

<img width="571" alt="Screenshot 2024-01-11 at 12 28 07 PM"
src="https://github.com/Qiskit/documentation/assets/14852634/2d915b94-ec48-445a-a174-ace628655b4b">

2. The link only takes you to the overall code page, not the specific
lines. This could be improved via
Qiskit#517.

But it's good enough to not block on these improvements.

This PR regenerates all Runtime historical versions, but not any current
versions nor Qiskit historical versions.

## How source code URLs are determined

`sphinx.ext.viewcode` embeds a copy of every Python file used in API
docs and uses internal relative links like
`../modules/qiskit_ibm_runtime/ibm_backend.html`. They correspond to
Python files we can be confident exist. We transform those relative
links into GitHub links here:


https://github.com/Qiskit/documentation/blob/486afd9b16be52dfedf63cdd357708884c99e110/scripts/lib/api/processHtml.ts#L94-L105


https://github.com/Qiskit/documentation/blob/486afd9b16be52dfedf63cdd357708884c99e110/scripts/lib/api/processHtml.ts#L163-L183

Our links assume that there is a branch called
`stable/<versionWithoutPatch` in GitHub, like `stable/0.8`.

## Replacing `[source]` with `GitHub ↗︎`

We remove the original link from Sphinx and replace it with our own.
This is important so that the link is not included in the `<code>` HTML
element incorrectly. It also allows us to set a custom link label and
`title` (the text when highlighting).
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Part of Qiskit#454. Historical
versions of Qiskit docs are tricky because of qiskit-metapackage
referring to multiple distinct GitHub repositories.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Part of Qiskit#454. Note that
Qiskit 0.45 refers to Qiskit Terra 0.25.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Part of Qiskit#454. Note that
Qiskit 0.43 refers to Qiskit Terra 0.24.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API docs epic 🏔️ infra 🏗️ user feedback 📥 General user feedback for design research purposes
Projects
Archived in project
Development

No branches or pull requests

5 participants