Match EEGLAB pop_prop channel and component properties figure

[innaamogolonova]

Bring pop_prop (Plot > Channel/Component properties) to EEGLAB parity: a scalp map, an ERP image reusing erpimage, and the activity power spectrum. Component spectra scale by scalp-map power (mapnorm), the component map centers its color axis on zero and shows electrodes with three contours, the selected channel is marked in red, and the spectrum y-axis hugs the plotted frequency band. Adds parity tests.

Visual parity (EEGLAB on the left):

[innaamogolonova]

@claude please review this PR which brings pop_plot to parity with EEGLAB. Highlight any points of concern that should be addressed before merge.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[innaamogolonova]

@claude please review this PR which brings pop_plot to parity with EEGLAB. Highlight any points of concern that should be addressed before merge.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[bruAristimunha]

🤖 @claude the workflow model id was stale (fixed in #310) — please review this PR now.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[bruAristimunha]

🤖 @claude retrying — diagnostic run to capture the workflow error.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[innaamogolonova]

@claude please review this PR which brings pop_plot to parity with EEGLAB. Highlight any points of concern that should be addressed before merge.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[innaamogolonova]

🤖 Code review — automated review of the pop_prop parity changes.

Summary

• Overall: Strong, parity-accurate, and well-tested. The core numeric claims were verified line-by-line against EEGLAB source. Two minor issues, neither a hard merge blocker.
• Highest-risk area: ERP-image panel labeling for components (user-facing parity).
• Merge recommendation: Safe to merge; worth fixing the component ERP y-label first (one line).

Parity verified against EEGLAB source ✅

• mapnorm — EEGLAB scales linear power by sqrt(mean(map⁴)) before 10*log10 (spectopo.m:350 → :372); the PR matches, and the test's +5*log10(mean(map⁴)) dB offset is correct.
• caxis fraction — EEGLAB's caxfraction resolves to [-f·max(|img|), +f·max(|img|)] (erpimage.m:2013-2018); the PR's ±f·max(|image|) matches.
• spectrum trace red — EEGLAB draws a single channel/IC spectrum with allcolors{2} = [1 0 0] (spectopo.m:166,552), so color="red" is correct parity.
• component map — numcontour=3 + electrodes-on + maplimits='absmax' all match pop_prop.m:164-165.
• Global offset subtraction, per-epoch spectrum, and continuous reshape all match; no dangling references after the _frequency_window/_spectra_ylim publicization, the _draw_channel_topo → plot_channel_location move, or the new backward-compatible mapnorm kwarg.

Important

1. Component ERP-image panel is labeled "µV", but IC activations are unitless — EEGLAB blanks it.

erpimage.py:125 always sets erp_ax.set_ylabel("µV"), and _draw_erp_image passes no y-label override for either channels or components. EEGLAB deliberately blanks this label for components — pop_prop.m:200/:233 call erpimage(..., 'yerplabel', '', ...) — precisely because component activations aren't in microvolts. As written, viewing IC properties shows a "µV" axis on the ERP average trace, which is misleading and a visible parity miss.

Fix: thread a blank yerplabel through _draw_erp_image → erpimage and pass "" for the component path (typecomp == 0). Small change; add a component-path assertion since test_erp_average_uses_global_offset_subtracted_data only checks the channel path today.

Nits

2. Figure is created before the index is validated — leaks an open figure on bad input.

In _plot_one_property, plt.figure(...) and the subfigures are built first; the index < 1 or index > … guards raise afterward, so an out-of-range channel/component leaves an un-closed figure (matplotlib warns past 20 open figures in an interactive session). EEGLAB validates the range (pop_prop.m:130) before creating the figure.

Fix: validate index against data.shape[0] / acts.shape[0] before plt.figure(...).

Test gaps

• Component ERP-panel label (tied to fix # 1) — no assertion currently covers the component yerplabel.
• Otherwise coverage is genuinely good: mapnorm offset, caxis fraction, target reuse, y-axis hug, symmetric color axis, electrode markers, continuous→erpimage, and numcontour are all directly tested.

EEGLAB parity notes

• Out of scope (pre-existing): winhandle is accepted and threaded into the history command, but the ACCEPT/REJECT/Values reject-buttons workflow (pop_prop.m:286-372, used when launched from pop_selectcomps) isn't implemented. This predates the PR and isn't part of the three-panel-figure goal — noting only so it's tracked, not something to fix here.

---

Areas checked: correctness/regressions in the diff, EEGLAB numeric parity (mapnorm, caxfraction, offset, spectrum color, contours, color axis), data-structure handling (epoched vs continuous, channel-major shapes, 1-based indexing), reference integrity after the renames/moves, and changed-behavior test coverage.

[innaamogolonova]

Addressed the two review items in 1b318a4:

• Component ERP y-label (important): threaded yerplabel through _draw_erp_image → erpimage. Components now blank the ERP ordinate label (unitless activations) and channels use EEGLAB's default "ERP" — matching pop_prop.m (channel :192 default, component :200 'yerplabel', ''). Note this differs from pop_erpimage, which uses µV; erpimage's default is left as-is so that path is unaffected.
• Figure leak (nit): the channel/component index is now validated before plt.figure(...), so a bad index no longer leaves an open figure (EEGLAB validates first, pop_prop.m:130).
• Tests updated to enforce the ERP label on both paths ("ERP" for channels, blank for components) plus a changelog note. pytest (pop_prop/erpimage/plot-wrappers) green; ruff + ty clean.

Updated screenshots:

[arnodelorme]

Good job. Could ERPimage be fixed so the ERP shows right below the image (bottom) as in the original (top)?