-
Notifications
You must be signed in to change notification settings - Fork 289
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 "Run in Colab" and "Download Notebook" links in tutorials #2268
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/2268
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 3 New Failures, 11 Pending, 5 Unrelated FailuresAs of commit 9b38cf6 with merge base ba6897d ( NEW FAILURES - The following jobs have failed:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
docs/source/_templates/layout.html
Outdated
@@ -61,7 +61,7 @@ | |||
if (downloadNote.length >= 1) { | |||
var tutorialUrl = $("#tutorial-type").text(); | |||
var githubLink = "https://github.com/pytorch/rl/blob/main/tutorials/sphinx-tutorials/" + tutorialUrl + ".py", | |||
notebookLink = $(".reference.download")[1].href, | |||
notebookLink = $(".reference.download")[0].href, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was that the ".reference.download" selects both the .py
and .ipynb
links, and the .ipynb
one happens to appear first in the html that sphinx-gallery generates. But I'm not sure what determines the order of the links--maybe it could change between different versions of sphinx-gallery.
We could potentially do something like this instead:
$(".sphx-glr-download-jupyter").find(".download.reference")[0].href
That would only select the .ipynb
, so the order of the links in the generated html wouldn't matter. However, I don't see anything about the "sphx-glr-download-jupyter" html class in the sphinx-gallery docs, so it could be unsafe to assume that that will always work. But then again, I don't know if the "download" and "reference" classes are meant for public use either.
I'm not sure what the sphinx-gallery docs say about how this is supposed to be done. I looked and was unable to find anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, there is an issue/PR in sphinx-gallery where "sphx-glr-download-jupyter" was added for public use: sphinx-gallery/sphinx-gallery#622
So it seems like it should be safe to use it. I'll update this PR
99d257f
to
1f0d282
Compare
1f0d282
to
9b38cf6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Victory!! Thanks a million
Description
Fixes the links to run tutorials in Colab and to download notebooks.
Motivation and Context
close #2134
Types of changes
What types of changes does your code introduce? Remove all that do not apply:
Checklist
Go over all the following points, and put an
x
in all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!