From 308ba06f37080ca5c24d36f11ea2b2cf7ad37961 Mon Sep 17 00:00:00 2001 From: Arnaud Delorme Date: Mon, 7 Sep 2026 19:47:08 -0700 Subject: [PATCH] Fix component colorbar tick placement and axes targeting --- .github/workflows/test.yml | 9 +++ functions/popfunc/pop_topoplot.m | 26 ++++---- tests/test_pop_topoplot_colorbar.m | 97 ++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 tests/test_pop_topoplot_colorbar.m diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1a14ca000..3d057d491 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -70,6 +70,15 @@ jobs: assert(EEG.nbchan == size(EEG.data, 1)); assert(EEG.pnts == size(EEG.data, 2)); + - name: Run MATLAB regression tests + if: steps.matlab.outputs.available == 'true' + uses: matlab-actions/run-command@v2 + with: + command: | + cd(getenv('GITHUB_WORKSPACE')); + results = runtests('tests'); + assertSuccess(results); + - name: Skip MATLAB smoke if: steps.matlab.outputs.available != 'true' run: | diff --git a/functions/popfunc/pop_topoplot.m b/functions/popfunc/pop_topoplot.m index a943e5958..942e2875b 100644 --- a/functions/popfunc/pop_topoplot.m +++ b/functions/popfunc/pop_topoplot.m @@ -430,22 +430,24 @@ % Draw colorbar if colorbar_switch + colorbarLimits = maplimits; + if ischar(colorbarLimits) + colorbarLimits = get(gca, 'clim'); + end if nbgraph == 1 - if ~ischar(maplimits) - ColorbarHandle = cbar(0,0,[maplimits(1) maplimits(2)]); - else - ColorbarHandle = cbar(0,0,get(gca, 'clim')); - end + ColorbarHandle = cbar(0,0,colorbarLimits); pos = get(ColorbarHandle,'position'); % move left & shrink to match head size set(ColorbarHandle,'position',[pos(1)-.05 pos(2)+0.13 pos(3)*0.7 pos(4)-0.26]); - elseif ~ischar(maplimits) - cbar('vert',0,[maplimits(1) maplimits(2)]); - else cbar('vert',0,get(gca, 'clim')); + else + ColorbarHandle = cbar('vert',0,colorbarLimits); end - if ~typeplot % Draw '+' and '-' instead of numbers for colorbar tick labels - tmp = get(gca, 'ytick'); - set(gca, 'ytickmode', 'manual', 'yticklabelmode', 'manual', 'ytick', [tmp(1) 0 tmp(end)], 'yticklabel', { '-' '0' '+' }); - try, icadefs; set(gca,'FontSize',AXES_FONTSIZE_L+2); catch, end + % Signed component labels require a scale spanning negative and positive values. + if ~typeplot && colorbarLimits(1) < 0 && colorbarLimits(2) > 0 + tickLimits = get(ColorbarHandle, 'ylim'); + zeroTick = tickLimits(1) - colorbarLimits(1)*diff(tickLimits)/diff(colorbarLimits); + set(ColorbarHandle, 'ytickmode', 'manual', 'yticklabelmode', 'manual', ... + 'ytick', [tickLimits(1) zeroTick tickLimits(2)], 'yticklabel', { '-' '0' '+' }); + try, icadefs; set(ColorbarHandle,'FontSize',AXES_FONTSIZE_L+2); catch, end end end diff --git a/tests/test_pop_topoplot_colorbar.m b/tests/test_pop_topoplot_colorbar.m new file mode 100644 index 000000000..e41800faf --- /dev/null +++ b/tests/test_pop_topoplot_colorbar.m @@ -0,0 +1,97 @@ +function tests = test_pop_topoplot_colorbar +tests = functiontests(localfunctions); +end + +function setupOnce(testCase) +root = fileparts(fileparts(mfilename('fullpath'))); +testCase.applyFixture(matlab.unittest.fixtures.PathFixture(root)); +testCase.applyFixture(matlab.unittest.fixtures.PathFixture(fullfile(root, 'functions'), 'IncludingSubfolders', true)); +testCase.TestData.EEG = pop_loadset('filename', 'eeglab_data_epochs_ica.set', 'filepath', fullfile(root, 'sample_data')); +end + +function setup(testCase) +testCase.TestData.figures = findall(groot, 'Type', 'figure'); +testCase.TestData.visibility = get(groot, 'DefaultFigureVisible'); +set(groot, 'DefaultFigureVisible', 'off'); +figure('Visible', 'off'); +end + +function teardown(testCase) +delete(setdiff(findall(groot, 'Type', 'figure'), testCase.TestData.figures)); +set(groot, 'DefaultFigureVisible', testCase.TestData.visibility); +end + +function testPositiveLimits(testCase) +checkScale(testCase, [1 2], 2, false); +end + +function testNegativeLimits(testCase) +checkScale(testCase, [-2 -1], 2, false); +end + +function testSymmetricLimits(testCase) +checkScale(testCase, [-2 2], 2, true); +end + +function testAsymmetricLimits(testCase) +checkScale(testCase, [-1 3], 2, true); +end + +function testZeroLowerEndpoint(testCase) +checkScale(testCase, [0 2], 2, false); +end + +function testZeroUpperEndpoint(testCase) +checkScale(testCase, [-2 0], 2, false); +end + +function testMultiplePositiveMaps(testCase) +checkScale(testCase, [1 2], [1 2], false); +end + +function testMultipleSymmetricMaps(testCase) +checkScale(testCase, [-2 2], [1 2], true); +end + +function testDefaultLimits(testCase) +pop_topoplot(testCase.TestData.EEG, 0, 2, 'Component', [], 0); +bar = findall(gcf, 'Type', 'axes', 'Tag', 'cbar'); +verifyNumElements(testCase, bar, 1); +verifyTrue(testCase, all(diff(get(bar, 'YTick')) > 0)); +verifyEqual(testCase, cellstr(get(bar, 'YTickLabel')), {'-'; '0'; '+'}); +end + +function testZeroComponent(testCase) +EEG = testCase.TestData.EEG; +EEG.icawinv(:, 2) = 0; +pop_topoplot(EEG, 0, 2, 'Zero component', [], 0); +bar = findall(gcf, 'Type', 'axes', 'Tag', 'cbar'); +verifyNumElements(testCase, bar, 1); +verifyTrue(testCase, all(diff(get(bar, 'YTick')) > 0)); +end + +function testUnrelatedAxesUnchanged(testCase) +other = axes('YTick', [10 20 30]); +figure('Visible', 'off'); +checkScale(testCase, [-2 2], 2, true); +verifyEqual(testCase, get(other, 'YTick'), [10 20 30]); +end + +function checkScale(testCase, limits, components, signed) +pop_topoplot(testCase.TestData.EEG, 0, components, 'Component', [], 0, 'maplimits', limits); +bar = findall(gcf, 'Type', 'axes', 'Tag', 'cbar'); +verifyNumElements(testCase, bar, 1); +ticks = get(bar, 'YTick'); +verifyTrue(testCase, all(isfinite(ticks)) && all(diff(ticks) > 0)); +labels = cellstr(get(bar, 'YTickLabel')); +if signed + verifyEqual(testCase, labels, {'-'; '0'; '+'}); + range = get(bar, 'YLim'); + mappedZero = limits(1) + (ticks(2)-range(1))/diff(range)*diff(limits); + verifyEqual(testCase, mappedZero, 0, 'AbsTol', 1e-12); +else + values = str2double(labels); + verifyTrue(testCase, all(isfinite(values))); + verifyEqual(testCase, values([1 end]), limits(:), 'AbsTol', 1e-12); +end +end