Merge develop into the browser epic - #403
Merged
neuromechanist merged 46 commits intoSep 21, 2026
Merged
Conversation
* Add provenance for current EEGLAB test ports * Add MATLAB fixture loader for test ports
* Port current pop regression tests * Port core popfunc conversion tests * Port ICA rejection helper tests * Port current pop utility workflows
* Port current EEGLAB admin and GUI tests * Remove internal implementation notes
* Port current EEGLAB sigprocfunc tests * Contain topoplot finite matrix warnings * style: format pop reref tests
* Port current dataset workflow tests * Port current pop_eegfilt test
* Port EEGLAB numerical utility tests * fix: harden numerical utility contracts
* feat: implement standalone LIMO workflows * fix: validate LIMO design cells
* test: adjudicate remaining EEGLAB methods * test: port current EEGLAB tutorial workflows * test: port face BIDS tutorial workflow * test: add EEGLAB port completeness audit * test: make live MATLAB validation exhaustive
…py (#397) * Fix the resampling ratio for non-integer sampling rates EEGLAB computes the ratio as rat(freq/EEG.srate, 1e-12), pop_resample.m line 119. This was ported as sympy.nsimplify, which is a different function: it looks for a simple symbolic expression rather than a rational one, and returns an irrational when no simple fraction is close enough. as_numer_denom then hands back a numerator that is not an integer and int() truncates it silently. A 2000 Hz recording resampled to 999.9 Hz was therefore resampled by 5/12 instead of 9999/20000, a 16.7 percent error, while the output was stamped with the rate that was asked for. The event latencies were rescaled by the same wrong ratio, so the result stayed internally consistent and gave the reader nothing to notice. Across realistic downsampling pairs, 24 of 86 came back wrong, all of them with a fractional source rate; the worst produced 117.04 Hz labeled 128 Hz. _rational_within expands the continued fraction one term at a time and stops at the first convergent inside the tolerance, which is what rat does and which returns the smallest denominator that will do. That matters beyond tidiness, because the denominator sets the anti-aliasing filter length: an earlier draft searched by doubling a denominator cap, which can overshoot the minimal answer and buy filter taps for nothing. The ratio tests fail 28 subtests against the previous implementation, and the end-to-end test added here fails against it too, computing 21/84 where the correct ratio is 2560/9999. An earlier draft of that test used whole-number rates only and passed against the defect, which made it worthless as a guard. Also logs a warning when the ratio's denominator is large. 128 Hz from a 512.03 Hz recording is 12800/51203, which builds a filter of about 1.64 million taps and costs seconds per minute of 64-channel data against milliseconds for a whole-number pair. That cost is EEGLAB's, inherited with its tolerance, and correct rather than a defect, but a resample that appears to hang should say why. * Drop sympy, which no longer has a caller The nsimplify the previous commit replaced was the only use of sympy anywhere in src/, so it leaves the base install. Verified by running the suite with sympy absent from the environment rather than merely absent from pyproject.toml, which is the only check that means anything here: eeglabio has no import site in src/ either, and is not removable, because mne imports it when asked to write an EEGLAB file and eeg_mne2eeg exports through mne. The uv.lock edit removes the two lines naming sympy in eegprep's own dependency lists, and is scoped to eegprep's package block. An earlier draft used an unscoped replace and took torch's sympy edge with it, orphaning the package entry: uv lock --check still passed, because the lock stayed self-consistent, and no CI job installs the torch extra with --locked, so nothing would have caught it.
… ships (#399) * Require Python 3.12 The browser target sets this, in the opposite direction to the obvious one. Pyodide 0.29.5 ships CPython 3.13.2 and its wheels are cp313, so requires-python is a ceiling rather than a floor: 3.14 would make eegprep uninstallable in the browser until Pyodide moves. 3.12 keeps one version of headroom and lets CI cover both 3.12 and the 3.13 the browser actually runs. The matrix drops 3.10 and 3.11 and gains 3.13. tomllib has been in the standard library since 3.11, so the tomli backport and its two try/except shims are dead code and go with it. * Raise dependency floors, bounded by what Pyodide ships Two of these are live bugs rather than tidying. threadpoolctl was declared >=3.6.0 while Pyodide 0.29.5 ships 3.5.0, so the floor is above what the browser can provide and a browser install cannot satisfy it. matplotlib>=3.9.2 would have introduced the same bug, since Pyodide has 3.8.4; 3.8.0 is the newest floor that does not. The rest were fiction in the other direction: numpy>=1.20 ships cp37 through cp39 wheels and cannot install on the Python this package now requires. Floors are now the first release with real 3.12 support, capped by the version Pyodide ships: numpy 2.1.0, scipy 1.14.1, matplotlib 3.8.0, h5py 3.12.1, threadpoolctl 3.5.0. scipy is split by platform. 1.14.1 is exactly what Pyodide has, so the general floor cannot go higher, but scipy built before 1.16.3 fails to dlopen on current macOS with a zero-fill section error, so darwin gets its own floor. * Pin every CI job to Python 3.12 The 3.12 floor landed in the test matrix but not in the jobs that install a single interpreter, so docs, release, ruff/ty, and the Claude workflows all still asked for 3.11 and uv refused: error: The requested interpreter resolved to Python 3.11.16, which is incompatible with the project's Python requirement: `>=3.12` Tested: the failing jobs were "Ruff and ty" and "Build Documentation". * Retire the last 3.10 and 3.11 references The floor moved but the surrounding claims did not: the README badge, the AGENTS notes, four docs pages, and the six example extensions all still advertised 3.10 or 3.11, and ty was still type-checking against 3.10 semantics. pre-commit.py carried a tomli backport for 3.10 and a matching "tomli is not installed; skipping TOML syntax checks" branch. On a 3.12 floor tomllib is always stdlib, so both are dead, and the branch was a silent skip of a check the script claims to run. Tested: ruff check and format clean, ty clean apart from three pre-existing unresolved torch imports (the local venv has no torch extra), tests/test_extension_catalog.py 22 passed, and ./pre-commit.py --all-files reports OK with config syntax checked. * Let CI test 3.13 where MATLAB cannot follow The 3.13 job failed on the MATLAB engine install, not on eegprep: MATLAB Engine for Python supports Python version 3.9, 3.10, 3.11, and 3.12, but your version of Python is 3.13 uv sync had already succeeded, so every dependency floor resolves on 3.13; only MathWorks' engine has no build for it. The workflow already treats MATLAB as optional and falls through to a plain pytest run when the engine will not start, but the install step aborted the job before reaching that check. Tolerate the install failing so the fall-through runs, which also means this starts working by itself once a MATLAB release covers 3.13, rather than encoding a compatibility table in CI. * Fix a parity-harness instruction the new floor broke development.rst told the reader to build the parity harness venv with --python 3.11 and then pip install -e this working tree into it. Under requires-python >=3.12 that install now fails, so the instruction was not merely stale, it no longer worked. Also split the two-sentence line in faq.rst onto its own lines, per the semantic line break convention. * Stop pre-commit reporting OK with YAML unchecked Removing the tomli backport fixed the TOML half of a false green: a missing parser printed one warning, skipped every file of that type, recorded no error, and let the run print OK. The YAML half was left behind with the identical shape. pyyaml is declared in this script's inline dependencies, so uv installs it before the body runs and the only way to reach the fallback is to start the script without uv. That is a misuse, not a degraded environment, so it now exits with the invocation to use instead of quietly checking nothing. Also corrects the header, which still said config parsing happens "when dependencies are available". It is now unconditional. Tested: ./pre-commit.py --all-files reports OK over 1221 files with config syntax checked. * Test that the floors stay installable in the browser Pyodide bundles its own build of every compiled package, so a floor above what a Pyodide release ships cannot be satisfied in the browser. That makes these floors and requires-python a ceiling, which is the opposite of how a floor reads and is invisible from the code. The epic branch's tools/check_pyodide_base_resolution.py was supposed to catch this and does not: it matches package names against the Pyodide lock and never compares versions, so threadpoolctl>=3.6.0 sits on that branch today against a distribution shipping 3.5.0, and the gate calls it ok. Filed as #400. These tests close the gap on develop, offline, against the versions Pyodide 0.29.5 bundles. Verified non-vacuous by reintroducing the real bug: threadpoolctl>=3.6.0 fails two of them by name, and a requires-python of >=3.14 fails the interpreter check, since Pyodide runs CPython 3.13.2. Also records the constraint above the floors it governs, which was documented for scipy alone, and bumps the ruff target to py312, missed when the ty target moved. The catalog fixture's python_requires went back to a value no interpreter bump can disturb; pinning it to the project floor coupled an unrelated happy path to every future bump. The field's real behavior, an unsatisfiable floor and a malformed specifier, now has the two tests its eegprep_requires sibling already had and it did not.
The epic branch was 45 commits behind develop and building on it meant building on a stale base. Seven files conflicted. pyproject.toml: took develop's Pyodide-bounded floors, but kept this branch's extras split. Develop still carries oct2py in the base install and taking its side wholesale would have undone phase/374. docs.yml and release.yml: kept this branch's ICLabel jobs, took develop's 3.12 pins. Four more 3.11 pins came in from this side without conflicting, in the sdist smoke test and the Pyodide harness job, and are bumped too. storage.py: both sides added to the same class. This branch's mutation tracking and develop's _BackingFile, _validate_backing_file and empty() are all kept. changelog.rst and ica_and_components.rst: both sides appended; kept both, and develop's newer API entries. uv.lock: regenerated rather than hand-merged. Two bugs the merge exposed, neither findable on this branch alone: MemmapData.__array_function__ returned NotImplemented for everything except copyto and put. Defining that method opts the type into numpy's dispatch protocol, where NotImplemented raises rather than falling back, so every other numpy function failed on a MemmapData. np.size(data) in pop_reref is one caller of many. Develop's #351 test covers it and this branch never had that test. It now delegates to the mapped array, unwrapping nested handles so np.concatenate([a, b]) does not dispatch straight back into itself. The tomli backport in check_pyodide_base_resolution.py is dead on a 3.12 floor and ty flagged its ignore directive. Tested: 3196 passed, 277 skipped, 0 failed with MATLAB skipped. Ruff check and format clean, ty clean with no diagnostics, pre-commit OK over 1221 files.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, reopen this pull request to trigger a review.
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
neuromechanist
merged commit Sep 21, 2026
e9f1017
into
epic/324-pyodide-browser
12 of 13 checks passed
This was referenced Sep 21, 2026
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
2 participants
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.
Why
The epic branch was 45 commits behind develop, and the browser work is being built on top of
it. Too many moving parts to keep stacking on a stale base.
It also meant the epic branch did not have #397's resample fix, #399's Python 3.12 floor, or any
of develop's
MemmapDatawork.Conflicts, and how each was resolved
pyproject.tomloct2pyin the base install; taking its side wholesale would have undonephase/374. Base is 14 deps,oct2py/psutil/pyedflibstay in extras..github/workflows/docs.ymliclabelextra, develop's 3.12 pin..github/workflows/release.ymlsrc/eegprep/functions/adminfunc/storage.py_BackingFile/_validate_backing_file/empty().docs/source/changelog.rstUnreleased; both kept.docs/source/api/ica_and_components.rstuv.lockFour more 3.11 pins came in from this side without conflicting and are bumped too: the sdist
smoke test in
release.ymland the Pyodide harness job intest.yml. Only the quoted MATLABerror message still contains "3.11".
Two bugs the merge exposed
Neither was findable on this branch alone, which is the argument for not letting it drift again.
1.
MemmapData.__array_function__broke every numpy function on a mapped array. It returnedNotImplementedfor everything exceptcopytoandput. Defining that method opts the type intonumpy's dispatch protocol, and there
NotImplementedraises rather than falling back, so:np.size(data)inpop_reref.py:297is one caller of many. Develop's#351mmo memmap paritytest covers this path and this branch never had that test. It now delegates to the mapped array,
unwrapping nested handles as well so
np.concatenate([a, b])does not dispatch straight back intoitself and recurse.
2. The
tomlibackport intools/check_pyodide_base_resolution.pyis dead on a 3.12 floor,and
tyflagged its now-unused ignore directive.Verification
ruff checkandruff format --checkcleanty checkclean, no diagnostics./pre-commit.py --all-filesOK over 1221 filesuv lock --checkin syncnp.size,np.mean,np.stack, andnp.concatenateon nestedhandles all work, and
np.copytostill advances the mutation revisionNote on threadpoolctl
I previously described
threadpoolctl>=3.6.0on this branch as a live browser-install bug. Thatwas wrong and is corrected in #402 and on #400: threadpoolctl publishes a
py3-none-anywheel,so micropip installs any version from PyPI. The floor moves to
>=3.5.0here only because that iswhat develop carries, not because the old value was broken.
Related: #324, #386, #399.