-
-
Notifications
You must be signed in to change notification settings - Fork 25
FIX: Strip path_to_docs from notebook URLs #345
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
Conversation
When path_to_docs is configured (e.g., 'lectures'), the pagename includes this prefix. However, when the notebook repository has a flat structure (no nb_path_to_notebooks configured), we should strip the path_to_docs prefix to avoid incorrect URLs like 'lectures/jax_intro.ipynb' when the correct URL should be 'jax_intro.ipynb'. This fix ensures that: - path_to_docs is stripped from pagename before constructing notebook URLs - nb_path_to_notebooks (if configured) is still correctly prepended - Works correctly for both flat and nested notebook repository structures Adds comprehensive test coverage for this scenario.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
=======================================
Coverage ? 45.21%
=======================================
Files ? 2
Lines ? 387
Branches ? 0
=======================================
Hits ? 175
Misses ? 212
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🎭 Visual Regression Test ResultsDetails
Skipped testsmobile-chrome › theme.spec.ts › Theme Features › f-string interpolation styling |
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.
Pull request overview
This PR fixes incorrect notebook URLs when documentation source files are in a subdirectory (path_to_docs) but the notebook repository has a flat structure. The fix strips the path_to_docs prefix from the pagename before constructing notebook repository URLs, ensuring the URLs point to the correct location in the notebook repository while maintaining backward compatibility.
Key changes:
- Modified
launch.pyto strippath_to_docsfrom pagename when constructing notebook URLs - Added comprehensive test coverage for the new path stripping functionality
- Ensured compatibility with both flat notebook repositories and those using
nb_path_to_notebooks
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/quantecon_book_theme/launch.py |
Adds logic to strip path_to_docs prefix from pagename before constructing notebook URLs, fixing malformed Colab/JupyterHub links |
tests/test_build.py |
Adds test_add_hub_urls_path_to_docs_stripping function with two test scenarios covering flat repos and repos with nb_path_to_notebooks |
Addresses Copilot review comment: adds explicit test case verifying that when pagename doesn't start with path_to_docs prefix, the pagename is used as-is without modification.
Problem
When
path_to_docsis configured (e.g.,lectures), Sphinx passes pagenames with this prefix (e.g.,lectures/jax_intro). If the notebook repository has a flat structure (nonb_path_to_notebooksconfigured), this causes incorrect URLs:Before:
https://colab.research.google.com/.../blob/main/lectures/jax_intro.ipynb❌After:
https://colab.research.google.com/.../blob/main/jax_intro.ipynb✅Solution
Strip the
path_to_docsprefix from the pagename before constructing notebook URLs. This ensures:nb_path_to_notebooks(if configured) is still correctly appliedChanges
launch.pyto strippath_to_docsfrom pagename before URL constructionTesting
Context
Fixes issue where PR previews showed malformed Colab links with incorrect paths. This occurred when the source documentation was in a subdirectory (configured via
path_to_docs) but the notebook repository had a flat structure.