Skip to content

Fix Pyodide analysis pipeline (harvest #194: dataKey routing, MEMFS, MNE 1.x)#220

Open
jdpigeon wants to merge 4 commits into
mainfrom
pyodide-analysis-fix
Open

Fix Pyodide analysis pipeline (harvest #194: dataKey routing, MEMFS, MNE 1.x)#220
jdpigeon wants to merge 4 commits into
mainfrom
pyodide-analysis-fix

Conversation

@jdpigeon

@jdpigeon jdpigeon commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Background

Harvests the still-valid fixes from the stale #194 (pyodide-fix) and re-applies them onto current main. #194 itself isn't mergeable: it was built on a superseded asset-serving base (HTTP server on :17173) that main replaced with the pyodide:// protocol, and it predates the buildMarkerRegistry marker-code fix (its loadEpochsEpic reverted to the broken 0-based event map). So this takes the logic, not the commits, and keeps buildMarkerRegistry intact. #194 can be closed once this lands.

The bug

The Clean/analysis pipeline crashed with getEpochsInfoEpic: Cannot read properties of undefined (reading 'map'). Root cause: requestEpochsInfo/requestChannelInfo relied on worker.postMessage returning data — native postMessage returns undefined; results actually come back on the message channel. Pre-existing, identical on the old main.

Fixes

  • dataKey routing (parallel to plotKey): the worker echoes dataKey and converts the PyProxy result to plain JS (structuredClone can't clone a PyProxy). pyodideMessageEpic routes epochsInfo → SetEpochInfo, channelInfo → SetChannelInfo. The two info epics + loadEpochsEpic are now fire-and-forget.
  • MEMFS epochs load: new fs:readFileAsBytes IPC + writeEpochsToMemfs stage .fif files in Pyodide's WASM FS (it can't read host OS paths).
  • plotPSD: raw.plot_psd()raw.compute_psd().plot() (MNE 1.2+ removed plot_psd).
  • loadTopoEpic: calls plotTopoMap, not plotTestPlot.
  • renderAnalyzeButton: shows when epochsInfo.length > 0 (not only when the data looked bad, drop >= 2).
  • eslint: ignore leftover src/renderer/utils/pyodide/** (pyodide.asm.js pegged the linter). Drops now-unused DEVICES/plotTestPlot/parseSingleQuoteJSON imports.

Kept buildMarkerRegistry in loadEpochsEpic#194 would have reverted it.

Skipped from #194: channel-index-0 fix (current loadERPEpic already handles 0 post-Emotiv-removal), saveEpochs paren (already closed on main), Prettier-only churn.

Test plan

  • npm run typecheck clean
  • npm run lint completes (no hang — the pyodide ignore)
  • npm test — 29/29 pass
  • In-app (needs a human — can't run Pyodide/WASM in CI): Load Dataset → epoch stats appear; Clean Data → epochs plot + -cleaned-epo.fif saved; Analyze → topo (SVG) + PSD render; channel selector incl. first channel renders ERP.

🤖 Generated with Claude Code

jdpigeon and others added 4 commits July 6, 2026 19:21
… PSD

Harvests the still-valid fixes from the stale PR #194 (pyodide-fix) and
re-applies them onto current main. #194 itself isn't mergeable — it was built
on a superseded asset-serving base (HTTP server) that main replaced with the
pyodide:// protocol, and it predates the buildMarkerRegistry marker-code fix
(its loadEpochsEpic reverted to the broken 0-based event map). So this takes
the logic, not the commits, and keeps buildMarkerRegistry intact.

The pipeline crash (getEpochsInfoEpic: "Cannot read properties of undefined
(reading 'map')") was because requestEpochsInfo/requestChannelInfo relied on
worker.postMessage returning data — it returns undefined; results come back on
the message channel. Fixes:

- dataKey routing (parallel to plotKey): worker echoes dataKey + converts the
  PyProxy result to plain JS (structuredClone can't clone a PyProxy);
  pyodideMessageEpic routes epochsInfo -> SetEpochInfo, channelInfo ->
  SetChannelInfo. The two info epics + loadEpochsEpic are now fire-and-forget.
- loadCleanedEpochs: stage .fif files in Pyodide MEMFS via new fs:readFileAsBytes
  IPC + writeEpochsToMemfs (WASM FS can't read host OS paths).
- plotPSD: raw.plot_psd() -> raw.compute_psd().plot() (MNE 1.2+ removed plot_psd).
- loadTopoEpic: call plotTopoMap, not plotTestPlot.
- renderAnalyzeButton: show when epochsInfo.length > 0 (not only when drop >= 2).
- eslint: ignore leftover src/renderer/utils/pyodide/** (pyodide.asm.js pegged
  the linter). Drops now-unused DEVICES/plotTestPlot/parseSingleQuoteJSON imports.

Skipped from #194: the channel-index-0 fix (current loadERPEpic already handles
0 correctly post-Emotiv-removal), saveEpochs paren (already closed on main), and
Prettier-only churn.

Typecheck clean, lint completes, 29/29 tests pass. Needs in-app Pyodide
verification (can't run WASM in CI/locally here). Also documents the lab.js
23.x datastore gotcha in learnings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL
…locally

load_data() reads the recorded CSVs from `js.csvArray` (the worker sets
self.csvArray before running it), but the `js` module was never imported in
that scope — a prior worker refactor dropped the global import that used to
make it available. In Pyodide, `js` must be explicitly imported. Import it
locally inside the `csv_strings is None` guard so the native-MNE tests, which
pass csv_strings, never touch the Pyodide-only module.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL
raw_epochs.plot(..., events=None) raised "events must be an instance of bool
or ndarray, got None" — MNE 1.x validates the events arg; False is the
no-events-overlaid default. Also close the returned Figure so the worker's
PyProxy→JS conversion doesn't try to structuredClone it back (DataCloneError);
this clean plot isn't routed to the UI. Same MNE-1.x-API-tightening class as
the plot_psd → compute_psd fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNEh5EFg4dSpRrfJ2ae4vL
@jdpigeon jdpigeon force-pushed the pyodide-analysis-fix branch from 09aab1a to b109fa5 Compare July 7, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant