ENH: add the JupyterLite notebook setup cell (JupyterLite split 4/5) - #14150
ENH: add the JupyterLite notebook setup cell (JupyterLite split 4/5)#14150natinew77-creator wants to merge 29 commits into
Conversation
Installs MNE into the browser kernel with piplite and patches what Pyodide does not provide: HTTP data fetching, the readers that expect a file on disk, and a few things that need OS threads.
It controls whether a dependency with no pure-Python wheel aborts the install or is reported at the end. It was never about version bounds, and since the move to Pyodide 314 there are none left to clear anyway.
teonbrooks
left a comment
There was a problem hiding this comment.
Added some comments to this and some clarifying questions. let me know if you have any questions about it.
| import os | ||
| import io | ||
|
|
||
| # lzma: try real stdlib first (Pyodide ships it); only mock if absent. The |
There was a problem hiding this comment.
is there a case where lzma does not get shipped? does the ImportError ever get raised?
There was a problem hiding this comment.
No, never on this kernel. Pyodide 314.0.0 stopped unvendoring stdlibs, so lzma is now bundled and the ImportError can't fire. The mock was for the old 0.29.3 kernel, so the 314 bump you suggested is what made it dead. Removed, 39 lines.
| return mne_data_path + "/" + _rel | ||
|
|
||
|
|
||
| def _lite_dir_reader(_orig): |
There was a problem hiding this comment.
we should use the pathlib library to help parse the filepaths below.
There was a problem hiding this comment.
Done, though I went a bit wider than the section you flagged: rather than convert each site, I centralised them into _lite_data_path and _lite_rel_to_data, which turns 25 pieces of ad-hoc string surgery into two helpers. Verified against the old logic on ten inputs including .. segments and lookalike directories.
| import mne.minimum_norm as _mne_minv | ||
| import mne.chpi as _mne_chpi | ||
|
|
||
| for _mods, _name, _arg in ( |
There was a problem hiding this comment.
I'm having some difficulty following the logic of the reader overrides. how are they being used here? could you walk me through what's happening?
I'm guessing the individual examples/tutorials used specific readers so you can't just do a replacement for the mne.io.read_raw function.
There was a problem hiding this comment.
Your guess is exactly right, the tutorials call read_raw_fif, read_epochs, read_forward_solution and so on directly, so one hook on read_raw never sees them. Nothing is on disk here and readers validate through _check_fname(must_exist=True), so each one is wrapped to fetch its file first. I've written that up above the table and at it, since the explanation was 184 lines from where you were reading.
There was a problem hiding this comment.
This for-loop over the "tier one table" looks straightforward enough, but I'm surprised at how much code is needed elsewhere to wrap all these other functions; I had (perhaps naively) hoped a simple decorator / wrapper function / low-level monkeypatch would have been enough for all of them to just work.
Can you help me understand? If you're replicating the folder hierarchy that we'd typically see in the mne_data dir, and exposing it in a way that jupyterlite can access (e.g. "local" so no CORS headers), why is so much extra machinery needed for wrapping, e.g., read_raw_brainvision?
There was a problem hiding this comment.
There is a general hook, _check_fname(must_exist=True), and it covers most readers with no wrapper each. Three cases escape it: one filename that means several files (a .vhdr names its .eeg and .vmrk inside itself, so fetching on open is already too late), code that probes with os.path.exists before any reader runs, and readers that skip validation. I've rewritten the comment above the table to say exactly that.
| mne.viz.plot_bem = _lite_plot_bem | ||
|
|
||
|
|
||
| # EXPERIMENTAL 3D: MNE's normal Brain/VTK stack can't load in WASM, so |
There was a problem hiding this comment.
could you separate this part into a separate file to be loaded? it will give us a clear delineation of what is pretty solid given the jupyterlite setup and what might be swapped out with future pyvista-js upstream fixes
There was a problem hiding this comment.
Done, it's _lite_setup_cell_3d.py now. Worth flagging that it wasn't one trailing block, there were two with the threads and matplotlib patches in between, and the second block reads plt and pyodide_plt_show from that middle section. So the order is base, then 3D, then renderer, and the wrapper says so.
| ordinary Python, so ruff lints and formats it; this module only reads that file | ||
| and exposes it as the string the browser kernel needs. | ||
|
|
||
| The docs build prepends it only to the notebooks copied into the JupyterLite |
There was a problem hiding this comment.
Do we add a note that the cell should be removed if the user downloads the notebook from JupyterLite and tries to run it locally?
There was a problem hiding this comment.
Good catch, that case wasn't covered. The docstring only described the opposite one, the downloadable .ipynb never getting the cell. It's now in both places: the docstring, and the cell itself so the reader sees it.
| # License: BSD-3-Clause | ||
| # Copyright the MNE-Python contributors. | ||
|
|
||
| # This file is notebook source rather than a module: it installs packages with |
There was a problem hiding this comment.
is there a reason this isn't a module? it would be nicer if this was ultimately a function call so that it would just be a few lines added to the notebook instead of all the overrides being added there.
if it's a matter of the async install call, you could await til it's done and run the remainder of the script.
There was a problem hiding this comment.
you're right that the async install isn't the blocker. Awaiting first and then calling works fine.
The blocker is where the module would live. doc/sphinxext/ is excluded from the wheel ([tool.hatch.build] exclude = ["/doc", ...]), so the browser can't import it. That means mne/.
I measured what would move. Of the 735 lines in the base cell, about 638 are staging this docs build's tutorial datasets over HTTP: the reader wrappers, the dataset path shims, the OSF guard. Only about 97 are generic Pyodide compat (multiprocessing, threads, tqdm, routing requests through pyodide.http). Putting the 638 into mne/ would ship "how mne.tools serves its tutorial data" inside the package everyone pip-installs.
So roughly three options:
- All of it into
mne/as asetup_jupyterlite()call. Four-line cell, but ~1100 lines in the library, and it would want tests the way ENH: add a vtk.js backend for MNE's 3D renderer (JupyterLite split 3/5) #14144 did. - Just the ~97 generic lines into
mne/. Real library value and easy to test, but the cell still carries the rest, so it doesn't really solve your complaint. - Keep it in
doc/and have the cell fetch and exec it. The cell already derives the docs root fromlocation.href, so this would be about 8 lines. Nothing inmne/, but tracebacks becomeFile "<string>", line 412and it would depend on ENH: wire the JupyterLite build into the docs (JupyterLite split 5/5) #14157 staging the file.
One argument for leaving it visible: in a teaching notebook, someone wondering why read_raw_fif is behaving oddly can scroll up and read it. Options 1 and 3 both trade that away.
I'd lean towards 2, but I'm happy with any of them. Which would you prefer?
There was a problem hiding this comment.
If this is only meant to be used in docs for now then I think having it live in doc/ is acceptable. It might be worth looking at how a library like scikit-learn (I think they use jupyterlite?) have gotten things to work, if/where they unit test, etc.
There was a problem hiding this comment.
Thanks, keeping it in doc/ then. I looked at scikit-learn, their cell is about 20 lines because their data arrives over the network, so pyodide_http.patch_all() covers it, whereas ours has to get files onto the filesystem before readers open them. They do not unit test it.
Worth borrowing anyway, and pyodide_http would replace my hand-rolled requests patch. It is not urgent though, pooch's downloader uses requests, which the patch already covers, and the one remaining urllib call (_get_latest_version) already degrades gracefully. So I would rather do it as a follow-up than swap an untested change onto a live path here.
| # .tar.gz archive, not just the extracted folder. Return the folder | ||
| # directly so pooch never tries to download from OSF. Return a Path | ||
| # (not a str) since tutorials use the / operator on the result. | ||
| from pathlib import Path as _Path |
There was a problem hiding this comment.
why are so many imports in this file getting aliased with a leading underscore? Is there a point to doing so?
There was a problem hiding this comment.
Dropped, all eleven. The rule is written at the top of the file now, module imports stay plain, and only names the cell invents get the prefix so they cannot shadow a tutorial's variables.
There was a problem hiding this comment.
Correction to the above: there were fourteen, not eleven, and three survived that round (_pyxdf, _tqdm, _mpb). All three are plain module imports now.
| import mne.minimum_norm as _mne_minv | ||
| import mne.chpi as _mne_chpi | ||
|
|
||
| for _mods, _name, _arg in ( |
There was a problem hiding this comment.
This for-loop over the "tier one table" looks straightforward enough, but I'm surprised at how much code is needed elsewhere to wrap all these other functions; I had (perhaps naively) hoped a simple decorator / wrapper function / low-level monkeypatch would have been enough for all of them to just work.
Can you help me understand? If you're replicating the folder hierarchy that we'd typically see in the mne_data dir, and exposing it in a way that jupyterlite can access (e.g. "local" so no CORS headers), why is so much extra machinery needed for wrapping, e.g., read_raw_brainvision?
| # /drive/ in Pyodide requires Cross-Origin-Isolation headers | ||
| # (COOP/COEP) which many static servers (e.g. CircleCI artifacts) | ||
| # do not send. Fetch the data over HTTP into /tmp/mne_data instead | ||
| # — same-origin, no CORS. The data is served at the docs root | ||
| # (/mne_data/...) via Sphinx html_extra_path. | ||
| # Pyodide may run in a web worker (no `window`); `location` exists | ||
| # in both the main thread and workers, so use it to find the docs | ||
| # root by splitting on '/lite/'. |
There was a problem hiding this comment.
I don't think I grasp the full picture here. Let me see if I can reconstruct it:
- a select set of data files are bundled as
lite_data, and pre-fetched from OSF.io in the setup cell (on every page?) - a handful of other files are lazy-fetched only when the notebook cell that needs them is executed by the pyodide kernel
- those files are placed in
/tmp/mne_data/? or/mne_data/? comment above is not clear about that. Also not clear where/drive/and/lite/come from. Are these all paths on a MEMFS file system? - how are things different between CircleCI and our regular website (which is served by GitHub pages)? (During build we have an env variable --- e.g.,
$"{CIRCLECI}" == "true"intools/get_minimal_commands.sh--- but that probably doesn't help for the built artifact?) - can we deploy some/all of the needed files to some path under
https://mne.tools/, to avoid needing to call out to OSF.io?
Not asking to do anything (at least not yet), just requesting help understanding the full picture and the limitations we're facing.
There was a problem hiding this comment.
- Nothing reaches OSF at runtime.
lite_datapulls from OSF at build time only; the cell fetches from the docs site itself,location.href.split("/lite/")[0] + "/mne_data/". OSF appears in the cell purely as a guard that turns an attempted download into a readable error. The eager list is 20 files, and yes, on every notebook. - Correct.
/mne_data/is the URL, served at the docs root byhtml_extra_path./tmp/mne_datais where the cell writes them, on Pyodide's MEMFS./lite/only locates the docs root./drive/is JupyterLite's own filesystem, which we deliberately avoid because it needs COOP/COEP headers.- No difference, and that is the point: avoiding
/drive/is what makes it behave the same on CircleCI artifacts and on GitHub Pages. - Already the case, per 1.
Your question made me measure the eager fetch: 42.9 MB per notebook, 22.4 MB of it lh/rh.pial and lh/rh.white. Those look redundant, since read_surface validates through _check_fname and the lazy hook should fetch them on demand. I have not moved them yet because _plot_mpl_stc reads surfaces through nibabel directly, bypassing that hook, so confirming it needs a real docs build.
The comment you were reading still claimed /drive/, which is what made this hard to follow. Fixed in 2231fd1.
There was a problem hiding this comment.
/mne_data/is the URL, served at the docs root byhtml_extra_path./tmp/mne_datais where the cell writes them, on Pyodide's MEMFS.
OK, so IIUC:
- at build time, files are fetched from OSF / OpenNeuro / wherever and served from e.g.
https://mne.tools/dev/mne_data/... - at run time, the setup cell copies the folder tree from
https://mne.tools/dev/mne_data/...into/tmp/mne_data/...on the MEMFS file system (but only for the files inlite_data? and the less-commonly-needed files are left athttps://mne.tools/dev/mne_data/...and only copied to the MEMFS when a cell actually needs one of them?)
There was a problem hiding this comment.
/mne_data/is the URL, served at the docs root byhtml_extra_path./tmp/mne_datais where the cell writes them, on Pyodide's MEMFS.OK, so IIUC:
- at build time, files are fetched from OSF / OpenNeuro / wherever and served from e.g.
https://mne.tools/dev/mne_data/...- at run time, the setup cell copies the folder tree from
https://mne.tools/dev/mne_data/...into/tmp/mne_data/...on the MEMFS file system (but only for the files inlite_data? and the less-commonly-needed files are left athttps://mne.tools/dev/mne_data/...and only copied to the MEMFS when a cell actually needs one of them?)
Both bullets are right. On your two question marks:
lite_data isn't the selector. It's just one of two places a file can come from at build time: _lite_src() checks ~/mne_data/<dataset>/ first, then falls back to the MNE-lite-data archive. It exists so the curated files are still there on a partial build. What actually gets served is decided by the lists in conf.py. The name points the wrong way here, sorry for the confusion.
So there are two lists, neither of them lite_data:
- served: 50 sample files, plus 25 from the other datasets.
- fetched at startup: the 20 in
_sample_files, a subset of those 50. That's the 42.9 MB.
And yes to your second one. Everything else stays at the URL until a cell asks for it. Nearly every MNE reader calls _check_fname(must_exist=True) before opening, so hooking that one function covers most of them. The few wrappers are for readers that open more than one file, or a whole folder.
Drops the underscore from the module aliases, uses args/kwargs and pathlib, and collapses two reader-table entries that were setting the attribute on the same module twice.
drammock
left a comment
There was a problem hiding this comment.
I think one of the things that is making this hard to review is the back-and-forth between helper functions and redefinitions of MNE functions/methods in _lite_setup_cell.py. It would be much clearer (to me) to have an ordering like:
- high-level explanation of why we need helper funcs
- defintions of all the helper funcs (_lite_data_path, _lite_rel_to_data, _lite_lazy_fetch, _lite_fetch_rel, _lite_folder_data_path, etc) with 1-2 line comments explaining each
- explanation of the 3 kinds of treatment that MNE funcs/meths need
- wrapping of the "easy" MNE funcs/methods (just reads 1 file)
- wrapping of the "medium" MNE funcs/methods (probes before read)
- wrapping of the "hard" MNE funcs/methods (reads multiple files)
| # Everything after the banner is what the notebook runs. The license header and | ||
| # the ruff directives above it belong to the file, not to the cell. |
There was a problem hiding this comment.
fix comment: there are no ruff directives in this file (anymore?)
There was a problem hiding this comment.
Reworded. The directives are in the files being read, not in this one, and the comment now says that.
| print("Fetching MNE sample data (once per session)...") | ||
| for _f in _sample_files: | ||
| _dst = _sample_dir + "/" + _f | ||
| if os.path.exists(_dst): |
There was a problem hiding this comment.
Gone, along with every other os.path in the file. Only os.environ is left.
| if _r.status != 200: | ||
| print(f" HTTP {_r.status} for {_url}") | ||
| continue |
There was a problem hiding this comment.
two comments here:
- when this gets actually connected to the sphinx build process, these
printstatements should probably be converted to sphinx log calls (https://www.sphinx-doc.org/en/master/extdev/logging.html) - it's a bit unexpected that we
continuewhen files fail to get fetched (likewise, wecontinuewhen the server returns HTML instead of the expected binary or text file, and catch any other errors without raising). Won't this lead to notebooks that can't execute due to missing data?
There was a problem hiding this comment.
The cell runs in the browser kernel, not the Sphinx process, so there is no logger to call; there is a note at the print now. On the second: you are right, it collects every failure and raises instead of continuing.
| os.makedirs(os.path.dirname(_cfg), exist_ok=True) | ||
| if not os.path.exists(_cfg): | ||
| with open(_cfg, "w") as _f: | ||
| _f.write("{}") |
There was a problem hiding this comment.
more os.path -> pathlib conversions here
| def _lite_kiloword_data_path(*args, **kwargs): | ||
| return _lite_lazy_fetch("MNE-kiloword-data", "kword_metadata-epo.fif") | ||
|
|
||
|
|
||
| mne.datasets.kiloword.data_path = _lite_kiloword_data_path | ||
|
|
||
|
|
||
| def _lite_erp_core_data_path(*args, **kwargs): | ||
| return _lite_lazy_fetch( | ||
| "MNE-ERP-CORE-data", "ERP-CORE_Subject-001_Task-Flankers_eeg.fif" | ||
| ) | ||
|
|
||
|
|
||
| mne.datasets.erp_core.data_path = _lite_erp_core_data_path | ||
|
|
||
|
|
||
| def _lite_mtrf_data_path(*args, **kwargs): | ||
| return _lite_lazy_fetch("mTRF_1.5", "speech_data.mat") | ||
|
|
||
|
|
||
| mne.datasets.mtrf.data_path = _lite_mtrf_data_path |
There was a problem hiding this comment.
Yes. Those three plus four more are now one factory and a 12-row table.
| return _p.relative_to(_mne_data_root).as_posix() | ||
|
|
||
|
|
||
| _sample_path = Path(_sample_dir) |
There was a problem hiding this comment.
this appears to be one-time-use; inline within the _lite_sample_data_path func
There was a problem hiding this comment.
Removed, and _lite_sample_data_path with it.
Group the fetch helpers together and split the reader overrides into the three kinds of treatment they need, per review. Along the way: read_events takes `filename`, not `fname`, and eegbci takes `subjects`, which 35_eeg_no_mri passes by keyword. The eager fetch now raises instead of continuing past a file the build did not stage.
Done in a2b3ae4. Helpers are grouped and documented in one section, then the overrides are split into your three groups. One thing to flag: there's a section between them holding the dataset shims, a twelve-row |
brain.screenshot() returned a blank 2x2 image, so 10_publication_figure cropped it and published two black squares as the real before/after. It raises now. Also: the requests shim reports the real HTTP status instead of always 200, so pooch fails on a 404 rather than on a later hash mismatch, and the plt_show comment no longer blames a bug MNE fixed in mne-toolsgh-14076.
It is fetch_fsaverage reached from montage.py, not fetch_infant_template: that one is only mentioned in prose by 25_automated_coreg, and the tutorial that really calls it is already excluded from the build.
montage.py was the last one, and it is back on the exclude list now that its rename is fixed. The shim stays: the list is the only thing keeping it unused, and a notebook added tomorrow would otherwise hit a Pyodide socket error instead of a real HTTP one.
Part 4 of the split of #13925. Parts 1 to 3 are #14128, #14135 and #14144.
Adds the cell prepended to every JupyterLite notebook. It installs MNE into the browser kernel with piplite, then patches what Pyodide does not provide:
requestsandpoochare routed throughpyodide.http, the readers that expect a file already on disk are wrapped to fetch it first, and a few things that need OS threads become no-ops.It appends the vtk.js renderer from #14144, so that one goes first.
The cell is a plain string constant and nothing in the build imports it yet. The Sphinx wiring that prepends it to notebooks is the last PR in the series.