Skip to content

[ENH] use contrast number in contrast filename #1347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Remi-Gau merged 6 commits into from
Mar 5, 2025
Merged

[ENH] use contrast number in contrast filename #1347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5e00c9
[ENH] use contrast number in contrast filename
Remi-Gau Feb 25, 2025
b163c1b
adapt test
Remi-Gau Feb 25, 2025
103cdd1
fix
Remi-Gau Feb 25, 2025
d7ce56a
add comment
Remi-Gau Feb 25, 2025
5773b38
fix
Remi-Gau Feb 25, 2025
e0b8cb6
fix
Remi-Gau Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

* [ENH] Add SPM's contrast number in filename of each contrasts output filename #1347 by @Remi-Gau

### Deprecated

### Removed
Expand Down
39 changes: 38 additions & 1 deletion demos/openneuro/models/model-ds000114_desc-testRetestVerbal_smdl.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,44 @@
"X": [
1
],
"Type": "glm"
"Type": "glm",
"Software": {
"bidspm": {
"Results": [
{
"name": [
"Task"
],
"p": 0.05,
"MC": "FWE",
"png": true,
"binary": true,
"threshSpm": true,
"nidm": true,
"montage": {
"do": true,
"orientation": "axial",
"slices": [
-26,
-22,
-18,
-14,
-10,
-6,
-2,
2,
6
],
"background": {
"suffix": "T1w",
"desc": "preproc",
"modality": "anat"
}
}
}
]
}
}
},
"DummyContrasts": {
"Test": "t"
Expand Down
2 changes: 1 addition & 1 deletion docs/source/output/outputs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ bidspm-stats
│   ├── spmT_0001.nii |
│   ├── spmT_*.nii ----------
│   |
│   ├── sub-blnd01_task-taskName_space-space_desc-contrastName_label-0039_p-0pt050_k-10_MC-FWE_montage.png
│   ├── sub-blnd01_task-taskName_space-space_label-0039_desc-contrastName_p-0pt050_k-10_MC-FWE_montage.png
│   |
│   ├── sub-blnd01_task-taskName_space-space_desc-afterEstimation_designmatrix.png
│   ├── sub-blnd01_task-taskName_space-space_desc-beforeEstimation_designmatrix.png
Expand Down
6 changes: 6 additions & 0 deletions src/stats/results/renameOutputResults.m
View file
Open in desktop
Copy link
Collaborator

@marcobarilari marcobarilari Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick check (I guess you know already but writing here a memo) and it seems at least in the moae demo that the label-contrastNb is not assigned because it is missing in the result structure.

Hardcoding result.contrastNb = 1; in here makes it work. I can try myself to move on but need some pointer or a hint on how to proceed.

Copy link
Contributor Author

@Remi-Gau Remi-Gau Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in CI the montage figure seems to have the label in the filename: https://github.com/cpp-lln-lab/bidspm/actions/runs/13524807083/job/37793493598?pr=1347#step:14:1087

I need to update CI so we can download and inspect the output of demo

Copy link
Collaborator

@marcobarilari marcobarilari Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sensed, indeed the montage and thresholded mask are named with the label but not the nidm or the table and glass brain page and it might be because those renamed take the contast nb from a different place compared to the other resutls (fixed by manually updatin result.contrastNb = 1)

Copy link
Contributor Author

@Remi-Gau Remi-Gau Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup: spend quite a bit of time trying to fix that yesterday but I did not find a clean way to fix that

Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,20 @@ function renameOutputResults(opt, result, subLabel)
end

if result.nidm
% TODO currently name of nidm file does not include
% the label of the contrast that was used to generate it.
renameNidm(opt, result, subLabel);
end

if result.png
% TODO currently name of PNG file does not include
% the label of the contrast that was used to generate it.
renamePngCsvResults(opt, result, '.png', subLabel);
end

if result.csv
% TODO currently name of CSV file does not include
% the label of the contrast that was used to generate it.

renamePngCsvResults(opt, result, '.csv', subLabel);

Expand Down
3 changes: 3 additions & 0 deletions src/stats/results/returnResultNameSpec.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
nameSpec.entities.sub = result.label;
nameSpec.entities.task = strjoin(opt.taskName, '');
nameSpec.entities.space = result.space{1};
if isfield(result, 'contrastNb')
nameSpec.entities.label = sprintf('%04.0f', result.contrastNb);
end
nameSpec.entities.desc = result.name;
nameSpec.entities.p = convertPvalueToString(result.p);
nameSpec.entities.k = num2str(result.k);
Expand Down
2 changes: 1 addition & 1 deletion src/workflows/stats/bidsResults.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@

for iBatch = 1:numel(matlabbatch)
batch{1} = struct('spm', matlabbatch{iBatch}.spm);
status = saveAndRunWorkflow(batch, batchName, opt, subLabel);
status = saveAndRunWorkflow(batch, [batchName, '_', num2str(iBatch)], opt, subLabel);

Check warning on line 229 in src/workflows/stats/bidsResults.m

View check run for this annotation

Codecov / codecov/patch

src/workflows/stats/bidsResults.m#L229 

Added line #L229 was not covered by tests

if status && isfield(matlabbatch{iBatch}, 'result')
renameOutputResults(opt, matlabbatch{iBatch}.result, subLabel);
Expand Down
4 changes: 2 additions & 2 deletions tests/tests_batches/stats/test_setBatchResults.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ function test_setBatchResults_export()
export{1}.png = true;
export{2}.csv = true;
export{3}.tspm.basename = ...
'sub-01_task-test_space-individual_desc-test_p-0pt050_k-0_MC-FDR_spmT';
'sub-01_task-test_space-individual_label-0001_desc-test_p-0pt050_k-0_MC-FDR_spmT';
export{4}.binary.basename = ...
'sub-01_task-test_space-individual_desc-test_p-0pt050_k-0_MC-FDR_mask';
'sub-01_task-test_space-individual_label-0001_desc-test_p-0pt050_k-0_MC-FDR_mask';

export{5}.nidm.modality = 'FMRI';
export{end}.nidm.refspace = 'subject';
Expand Down
1 change: 1 addition & 0 deletions tests/tests_slow/tests_workflows/stats/test_bidsResults.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function test_bidsResults_subject_lvl()
assertEqual(matlabbatch{1}.spm.stats.results.conspec, expected);

rootBasename = ['sub-ctrl01_task-vislocalizer_space-IXI549Space', ...
'_label-0003', ...
'_desc-VisMotGtVisStat_p-0pt050_k-5_MC-FWE'];
expected = {struct('png', true), ...
struct('csv', true), ...
Expand Down
Loading