Repository navigation
Prepare release v1.5.1 - #1023
Merged
Merged
Conversation
Bump version from 1.5.0 to 1.5.1 and split the post-v1.5.0 changelog entries (BandRotation, EMG2QwertyNet SpecAugment + return_features, AmplitudeScale RNG crash fix, Jonathan Dan author link) into a new 'Current 1.5.1 (stable)' section, mirroring the pattern from the v1.5.0 release commit.
Contributor
There was a problem hiding this comment.
Pull request overview
Prepares the braindecode v1.5.1 patch release by updating the package version and stamping the changelog so the post-v1.5.0 entries are grouped under a new “Current 1.5.1 (stable)” section.
Changes:
- Bump package version from 1.5.0 → 1.5.1.
- Reorganize
docs/whats_new.rstto add a new Current 1.5.1 (stable) section with enhancements + bug fixes (and link PR references via:gh:).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/whats_new.rst | Adds “Current 1.5.1 (stable)” section and moves/annotates release notes for the 1.5.1 patch release. |
| braindecode/version.py | Updates __version__ to 1.5.1 (used by pyproject dynamic version). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR #993 (1.5.0) removed the per-batch ch_names kwarg from Labram.forward and tightened the __init__ chs_info check into a ValueError. Downstream wrappers (neuroai's _LabramChannelWrapper) were still calling Labram with non-canonical chs_info and a per-batch ch_names subset, producing 7 test failures. Restore the kwarg as keyword-only (via *); when provided it case-insensitively maps each name to LABRAM_CHANNEL_ORDER and uses those indices for the position-embedding bank. When None, fall back to the 1.5.0 arange-over-canonical behavior. The __init__ canonical check is downgraded to a UserWarning so wrappers can build the inner Labram with their union channel set and resolve the subset per batch. Update test_labram_rejects_non_canonical_chs to assert the warning (renamed to test_labram_warns_on_non_canonical_chs). Document the restoration under 1.5.1 'API and behavior changes' in whats_new.
…-of-raise Since 1.5.1 downgraded Labram's non-canonical-chs_info ValueError to a UserWarning (to keep wrappers like neuroai's _LabramChannelWrapper working), update plot_channel_interpolation.py to demonstrate the warning behavior instead of catching the exception. The 'pretrained model expects a specific layout' narrative still holds — vanilla Labram still mis-aligns position embeddings on arbitrary user data when ch_names is not provided per batch.
Apply review feedback: - Lift the ch_names -> canonical-index dict to module scope as _LABRAM_CANONICAL_INDEX instead of rebuilding it every forward call (~128 string ops + dict inserts saved per batch). - Strip changelog narrative (the 'Restored in 1.5.1' / neuroai mentions) from the forward docstring and __init__ comment — that history lives in whats_new.rst, not in the runtime code. - Tighten the non-canonical-chs_info warning: drop Sphinx :meth: / :class: roles (they don't render at runtime) and collapse to 4 lines. - Drop the redundant 'x is already in canonical order' comment in the forward fallback branch — the arange already says it. - Trim the length-mismatch ValueError to the equation alone. Behavior unchanged; all 25 braindecode labram tests + 13 neuroai test_labram tests still pass.
Shipped-release changelog entries are append-only in this repo; the 1.5.1 entry already describes the ch_names restoration so the cross-reference in the 1.5.0 entry was redundant and broke convention.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
braindecode/models/labram.py:722
forwardintroduces a*beforech_names, which also makesreturn_patch_tokens,return_all_tokens, andreturn_featureskeyword-only. If the intent is only to makech_nameskeyword-only (as stated in the changelog), move the*so existing positional calls for the return flags remain supported, or update the docs to reflect the broader API change.
This issue also appears in the following locations of the same file:
- line 731
- line 751
- line 756
def forward(
self,
x,
*,
ch_names: list[str] | None = None,
return_patch_tokens=False,
return_all_tokens=False,
return_features=False,
):
braindecode/models/labram.py:755
- When
ch_namesisNone,input_chansis always built for the full canonical 128-channel bank. Ifx.shape[1] != len(LABRAM_CHANNEL_ORDER)(e.g., constructingLabramwith a smallerchs_infonow only emits a warning), this will later produce a token/position-embedding shape mismatch and fail at runtime. Consider adding an explicitValueErrorhere whench_names is Nonebutxis not canonical-sized (and point users toch_namesorInterpolatedLaBraM).
if ch_names is None:
input_chans = torch.arange(
len(LABRAM_CHANNEL_ORDER) + 1, device=x.device, dtype=torch.long
)
else:
braindecode/models/labram.py:737
ch_namesis documented as selecting channel position embeddings, but it is only applied inforward_featureswhenself.neural_tokenizerisTrue; in decoder mode the code path ignoresinput_chans. To avoid confusing callers, consider either raising/ warning whench_namesis passed in decoder mode, or documenting thatch_namesis supported only in tokenizer mode.
ch_names : list of str, optional
Channel names matching the channel axis of ``x``. Matched
case-insensitively against :data:`LABRAM_CHANNEL_ORDER` to
select the corresponding position embeddings, so callers can
forward an arbitrary subset of canonical channels. If
``None`` (default), ``x`` is assumed to already be in
:data:`LABRAM_CHANNEL_ORDER`.
braindecode/models/labram.py:771
- The new
ch_namesmapping logic (case-insensitive matching, unknown-name error path, and the new behavior whench_namesis omitted) is not exercised by unit tests. Adding focused tests for: (1) successful subset forward withch_names(including mixed-case names), and (2) a clear error whench_namescontains an unknown channel would help prevent regressions.
if len(ch_names) != x.shape[1]:
raise ValueError(
f"len(ch_names)={len(ch_names)} != x.shape[1]={x.shape[1]}"
)
try:
matched = [_LABRAM_CANONICAL_INDEX[n.upper()] for n in ch_names]
except KeyError as exc:
raise ValueError(
f"ch_names contains a name not in LABRAM_CHANNEL_ORDER: "
f"{exc.args[0]!r}. Filter unknown channels before calling "
f"forward, or use InterpolatedLaBraM."
) from exc
# CLS token at index 0; canonical channel indices are offset by 1.
input_chans = torch.tensor(
[0] + [i + 1 for i in matched], device=x.device, dtype=torch.long
)
- forward(): move `*` after the return_* flags so only `ch_names` is keyword-only. `return_patch_tokens`, `return_all_tokens` and `return_features` stay positional, preserving back-compat for callers that used positional args before #993. - forward(): when `ch_names is None`, validate `x.shape[1] == len(LABRAM_CHANNEL_ORDER)` up front and raise a clear ValueError pointing to `ch_names=` or `InterpolatedLaBraM` instead of failing later with a confusing shape mismatch inside `forward_features`. - forward() docstring: clarify that `ch_names` is keyword-only and only honored when `neural_tokenizer=True`; in decoder mode the position embedding is sequential. - examples/plot_channel_interpolation: drop the redundant in-body `import warnings` (already imported at module top) and rewrite the "silently mis-align" wording — the model warns at construction and forward now raises ValueError on a non-canonical channel count. - test_foundation_models: add focused tests for the ch_names path — subset forward, case-insensitive matching, unknown-channel ValueError, length mismatch, the new None+non-canonical guard, and back-compat for positional return_* flags.
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.
Summary
Patch release for braindecode 1.5.1, 11 days after v1.5.0.
braindecode/version.pyfrom1.5.0→1.5.1Current 1.5.1 (stable)section indocs/whats_new.rst, mirroring the pattern from PR Prepare release v1.5.0 #1010.What's in 1.5.1
Enhancements
BandRotationaugmentation for surface-EMG wristband layouts (augmentation: add BandRotation for surface-EMG wristband layouts #1013)EMG2QwertyNetbuilt-inspec_augment+return_features/return_featureflags for downstream wrappers ([ENH] EMG2QwertyNet: built-in SpecAugment + feature-extraction flags #1015)Bug fixes
AmplitudeScalecrashing on defaultrandom_state=Noneand onnumpy.random.RandomState(augmentation: Fix AmplitudeScale crashing on default random_state + more #1021)whats_new.rst(docs: fix broken contributor link #1022)Test plan
python -m build --sdistproducesbraindecode-1.5.1.tar.gzmake htmlPost-merge checklist
braindecode.github.io/1.5/and refreshstable/python -m twine upload --repository braindecode dist/*v1.5.1and create the GitHub release