short_path: write the graph with %%file, and move the cell so it can run - #63
Merged
Conversation
❌ Deploy Preview for beautiful-dodol-cb9543 failed.
|
There was a problem hiding this comment.
Pull request overview
This PR makes the short_path lecture fully self-contained for in-browser execution by embedding the synthetic graph data directly in the notebook content, eliminating the prior cross-repo network fetch and removing an unused local copy.
Changes:
- Replace the
requests.get(...)fetch ofgraph.txtwith an inline triple-quotedgraph_file_dataliteral (keeping downstream parsing/usage intact). - Remove the
print(graph_file_data)output to avoid duplicating the ~100-line graph listing in rendered output. - Delete
lectures/graph.txtsince it is no longer referenced.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lectures/short_path.md | Inlines the graph data as graph_file_data and removes the network fetch/printing while preserving the existing parsing and algorithm calls. |
| lectures/graph.txt | Removes the now-unused committed copy of the graph data. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (9e033ea) ✨ Browse the preview at the URL above. |
mmcky
force-pushed
the
datasets/graph-txt-inline
branch
from
August 12, 2026 02:20
9e033ea to
15e1b58
Compare
mmcky
changed the title
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (15e1b58) ✨ Browse the preview at the URL above. |
mmcky
force-pushed
the
datasets/graph-txt-inline
branch
from
August 12, 2026 02:38
15e1b58 to
089cacf
Compare
mmcky
changed the title
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (089cacf) ✨ Browse the preview at the URL above. |
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (68599a8) ✨ Browse the preview at the URL above. |
mmcky
force-pushed
the
datasets/graph-txt-inline
branch
from
August 12, 2026 02:57
68599a8 to
089cacf
Compare
This was referenced Aug 12, 2026
BLOCKED on #64 — do not merge yet. See that issue and the diagnostic in #65. This PR is the intended END STATE; what is unproven is whether this runtime can run it. This lecture executes in the reader's browser (`lite: true`) and was the only repo in the family fetching this graph across repos on every page view: import requests file_url = "https://raw.githubusercontent.com/QuantEcon/lecture-python-intro/main/lectures/graph.txt" graph_file_response = requests.get(file_url) Every sibling — lecture-python-intro, lecture-dp, lecture-jax, lecture-intro.zh-cn and test-actions-lecture-intro — writes the data with a `%%file graph.txt` cell and reads it back with `open(in_file)`. This repo now does the same, so all six are identical on this exercise, and the divergent string-splitting helper `map_graph_to_distance_matrix(in_file_data)` returns to the canonical filename form. It also retires a cross-repo dependency: that `requests.get` was the only thing reading lecture-python-intro's committed `lectures/graph.txt` anywhere in the organisation, so intro could not delete its copy while this line stood. This repo's own byte-identical `lectures/graph.txt`, which nothing read, is deleted here too. WHY THIS IS BLOCKED, and what is NOT known. `short_path` cannot be completed in-browser today, and this PR does not change that. `myst-ext-exercise` nests a gated `{exercise-start}` body as children of the `exercise` node — `exercise-start` is an ALIAS for `exercise`, not a hoist — and thebe only wires up cells at the top-level block layer. So the data cell sits at ['root','block','exercise','block'] and never executes, whichever form it takes. On `main` two cells are stranded there; with this change it is one. A second bug currently masks a worse version of that one. `short_path.md:480` is a bare ```{solution}``` closer that mystmd rejects, so the solution node is empty and its five cells spill to root level — which is the ONLY reason they are runnable. Correcting it to the gated pair was tried and rebuilt: the error clears, the cells correctly re-nest, and the page would go from one dead cell to six. So that fix is deliberately NOT included here. RETRACTED, and recorded so it is not repeated: an earlier revision of this PR asserted that `%%file` does not fire under thebe-lite. That was wrong. The evidence was a cell producing no output and a later FileNotFoundError, both of which are equally explained by the cell never having executed — which is what was happening. `%%file` under thebe-lite has never been tested. #65 tests it at top level, where nesting cannot confound the result. Equivalence of the data itself is measured, not assumed: intro's canonical filename-based helper over the file written by this cell gives 286 finite entries, Q[99,99]=0.0 and J[0]=160.5500, identical to the network path this replaces, and the written file is byte-identical at 4,691 bytes, sha256 046759fb... graph.txt is not migrating to QuantEcon/data-lectures: it is synthetic teaching data with null provenance, the exercise prose depends on it being visible, and that repo is a cache for upstream sources rather than a host for our examples.
mmcky
force-pushed
the
datasets/graph-txt-inline
branch
from
August 12, 2026 03:08
089cacf to
03bccc1
Compare
mmcky
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (03bccc1) ✨ Browse the preview at the URL above. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
A code cell nested in any directive is never registered with the kernel by
@myst-theme/jupyter, so inside `{exercise-start}` this cell renders a run button
that does nothing, and the solution then fails with FileNotFoundError. It is the
only cell on the page in that position — every other cell is already top level
and runs.
Moving it below `{exercise-end}` makes the whole lecture runnable in the
browser for the first time. Reading order is unchanged: the prose still says
"The text below describes a weighted directed graph" and the data still appears
directly beneath it, just outside the exercise's box. The solution's "reads in
the graph data above" is likewise still accurate.
A comment records why it sits there, so nobody tidies it back in — that would
silently break the page again, with no error and no CI signal.
Root cause and the general fix: #64 and
QuantEcon/quantecon-theme.mystmd#117.
Verified the move did not touch the data: 9 cells, 100 data lines,
reconstitutes to 4,691 bytes / sha256 046759fb..., and the cell now sits
strictly between `{exercise-end}` and the solution.
mmcky
changed the title
|
📖 Netlify Preview Ready! Preview URL: https://pr-63--beautiful-dodol-cb9543.netlify.app (bdabaf1) ✨ Browse the preview at the URL above. |
This was referenced Aug 12, 2026
mmcky
added a commit
to QuantEcon/data-lectures
that referenced
this pull request
Aug 12, 2026
* Record graph.txt's disposition now that wasm embeds it QuantEcon/lecture-wasm#63 stopped lecture-wasm fetching lecture-python-intro's committed graph.txt over the network and gave it a `%%file` cell like every sibling repo. That was the only URL read of that blob anywhere in the organisation, so three records here are now wrong. `scripts/audit_annotations.yml`: - the graph.txt note said lecture-wasm "fetches intro's committed copy by URL (requests.get), making that copy load-bearing". No longer true. Every consumer now embeds the data in its own lecture, and graph.txt has dropped out of the scanned dataset set entirely — it is `embedded` in four repos and a shadowed orphan in the rest. - the `lecture-wasm:lectures/graph.txt` mirror-orphan entry names a file that #63 deleted. Dead key, removed. - intro's copy became a BARE orphan the moment that read went away — `kind: orphan`, no note — which reads as an unexplained stray file. It is annotated `shadowed`, matching lecture-dp's identical case: short_path regenerates it via `%%file` before reading it, so the committed bytes are never consumed (the build prints "Overwriting graph.txt" where lecture-jax, which commits no copy, prints "Writing graph.txt"). `PLAN.md`: Track A is complete. The last two CSVs landed as wave A4 (#74, flipped in #75), and graph.txt is recorded as a deliberate NON-migration rather than a remaining item — synthetic teaching data, null in every provenance field, and the exercise teaches its format by quoting the first line, so the data has to stay visible on the page. Hosting it here would have put a toy in a registry that exists to carry provenance. The note also carries the deletion caveat, because the obvious next step is wrong at scale: intro's copy is deletable as Track X, but the same blob sits in 8 repos and is regenerated at 17 `%%file` sites including archived `.rst` ancestors that `gh search code` cannot see. That deletion needs a per-repo reader sweep, not an org-wide one. Figures re-read from audit.json rather than carried: 40 static files (was 41 — graph.txt is no longer a scanned dataset), 38 committed files (was 43 — five deletions across wasm and intro), 24 orphans, 26 migrated. Strict audit exit 0, zero warnings, zero problems in all four categories; catalog freshness gate green. * Copilot: distinguish 'shadowed' from 'embedded', and fix a contradiction in rule 2 Two review comments, both correct, plus a third instance of the first one that the review did not flag. **'shadowed everywhere' was wrong** (audit_annotations.yml). Shadowed means a committed copy exists and is overwritten by the %%file cell before being read. That is true of two of the four embedding repos, not all of them: lecture-python-intro %%file + committed copy -> shadowed lecture-dp %%file + committed copy -> shadowed lecture-jax %%file, no copy -> just embedded lecture-wasm %%file, no copy -> just embedded The note implied a committed copy exists everywhere, which is backwards for the two repos that model the shape we actually want. Reworded to split the cases. **PLAN.md rule 2 contradicted the rest of the document.** It still read "(graph.txt is Track A but single-consumer: only lecture-wasm reads intro's committed copy.)" — false twice over after this PR, since wasm no longer reads it and it is not a Track A item. Rule 2 is where someone scoping a repoint set looks, so the parenthetical is rewritten to say graph.txt is out of scope and why, rather than deleted. **The same imprecision was in PLAN.md's own disposition note**, which the review did not comment on: "embedded in four repos and a shadowed orphan in the rest" puts intro and dp in both sets. Now states which repos embed, which of those also commit a copy, and that the remaining committed copies (lecture-intro.zh-cn, the canary, lecture-python.zh-cn, lecture-dp.monorepo, ipynb_pdf_constructor) are read by nothing. Verified against the repos rather than restated: all four embed via %%file; only intro and dp commit a copy. Strict audit exit 0 with zero warnings, catalog gate green, audit_annotations.yml parses with the dead wasm key gone and the intro key present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes
short_pathrunnable in the browser, and retires this repo's last cross-repo data dependency.What changes
1. The graph is written with
%%file, as every other repo does.This lecture fetched the data across repos on every page view:
lecture-python-intro,lecture-dp,lecture-jax,lecture-intro.zh-cnandtest-actions-lecture-introall use a%%file graph.txtcell and read it back withopen(in_file). This repo now does the same, so all six are identical on this exercise and the divergent string-splitting helper returns to the canonical filename form.2. The data cell moves out of the exercise, so it executes.
A code cell nested in any directive is never registered with the kernel, so inside
{exercise-start}it renders a run button that does nothing — and the solution then fails withFileNotFoundError. It was the only cell on the page in that position; everything else was already top level. Moving it below{exercise-end}makes the lecture run end to end.Reading order is unchanged — the prose still says "The text below describes a weighted directed graph" and the data still appears directly beneath it, just outside the exercise's box. A comment records why it sits there, so it is not tidied back in.
Root cause and the general fix: #64 and QuantEcon/quantecon-theme.mystmd#117.
3. Two files are deleted.
lectures/graph.txthere, which nothing read — this repo shipped its own copy and downloaded intro's. And with therequests.getgone,lecture-python-intro's committedgraph.txtloses its only reader anywhere in the organisation and can be retired separately.Why the data is not migrating to data-lectures
graph.txtis synthetic teaching data —provenance: toy, null in every real provenance field. The exercise teaches its format by quoting the first line and saying "other lines have a similar interpretation", andnum_nodes = 100/destination_node = 99appear nowhere in prose, so the visible final linenode99,is the reader's only evidence for them. The data has to stay on the page.QuantEcon/data-lecturesis a cache for upstream sources, not a host for our own examples.Verification
CI here cannot check any of this —
ci.ymlrunsmyst build --htmlwith no execution — so it was measured directly.Equivalence, using intro's canonical helper over the file this cell writes:
Q[99,99]J[0]046759fb…— byte-identical to intro's copyThe move did not touch the data: 9 cells, 100 data lines, reconstitutes to the same bytes, and the cell now sits strictly between
{exercise-end}and the solution.Every executable cell on the page is now top level, confirmed from the built artifact rather than the source.
Please confirm on the preview
Run the cells top to bottom. Expect the cost-to-go output ending 160.55 — the first time this lecture has been completable in the browser.
import requestsis gone from the lecture entirely, soshort_pathnow runs with zero network calls and one fewer wheel for Pyodide to pull. That also removes the unshimmed read tracked in #59.