ENH: separate anatomical and functional reports per session for densely sampled dataset #3191

celprov · 2023-12-21T18:23:43Z

enh: beyond a certain number of session for a single subject (4), we separate the anatomical report from the functional and we separate the functional reports per session. For fewer sessions than 4, we keep the original behavior of aggregating all sessions and the anatomical in one report.

enh: find the suitable bootstrapfile within generate_reports rather than before

enh: add session_list argument to generate_reports

Builds on top of #3177

Closes #3080

… anatomical report and the functional reports per session. fix: let generate_reports() define the default config file

codecov · 2023-12-21T18:39:26Z

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (4d21c37) 71.90% compared to head (7884e58) 72.60%.

Files	Patch %	Lines
fmriprep/reports/core.py	76.31%	9 Missing ⚠️
fmriprep/cli/run.py	0.00%	1 Missing ⚠️
fmriprep/cli/workflow.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3191      +/-   ##
==========================================
+ Coverage   71.90%   72.60%   +0.69%     
==========================================
  Files          55       56       +1     
  Lines        4161     4241      +80     
==========================================
+ Hits         2992     3079      +87     
+ Misses       1169     1162       -7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…-nireports' into enh/sep_anat_func_report

…_report

celprov · 2024-01-19T09:55:07Z

@oesteban @effigies I merged #3177 into this PR. Please review it. Thank you!

fmriprep/reports/core.py

…port as a cli option

…p into enh/sep_anat_func_report

fmriprep/cli/parser.py

fmriprep/reports/core.py

fmriprep/cli/run.py

fmriprep/cli/workflow.py

fmriprep/reports/core.py

fix: turn the subject into a list if it is inputted as a string

… the name of the html reports enh: add test to verify that the reports get separated above max-ses-agr and that otherwise we retain the old behavior enh: add test to verify that the implementation above is not disrupted by adding a crash file in the subject folder

…session, the full boilerplate (describing both anatomical and functional preprocessing) is appended only in the anatomical report. enh: report errors in the visual report. Similarly to the boilerplate, in case of report separation, all errors are reported only in the anatomical report. fix: delete misplaced test files

…n the report. enh: add test to check that the boilerplate has been incorporated in the reports. fix: condense all the tests in one implementation

celprov · 2024-01-30T13:50:30Z

I don't know what to do about the "Check fmriprep" test, because if I run python -m black --color fmriprep locally, it does make any changes. Additionally, I tried applying those changes by hand, but it keeps complaining about the same lines requesting the same change again (in a vicious loop ^^').

PS the lines it complains about are not in the file I have changed for this PR

oesteban

Very quick lookthrough

fmriprep/cli/parser.py

fmriprep/cli/run.py

fmriprep/cli/parser.py

fmriprep/data/reports-spec-anat.yml

fmriprep/data/reports-spec.yml

oesteban · 2024-01-30T14:11:33Z

But certainly not under the reportlets dir...

…

On Tue, Jan 30, 2024 at 3:10 PM celprov ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In fmriprep/data/reports-spec-anat.yml <#3191 (comment)>: > + - bids: {datatype: figures, space: .*, suffix: T1w, regex_search: True} + caption: Spatial normalization of the T1w image to the <code>{space}</code> template. + description: Results of nonlinear alignment of the T1w reference one or more template + space(s). Hover on the panels with the mouse pointer to transition between both + spaces. + static: false + subtitle: Spatial normalization of the anatomical T1w reference + - bids: {datatype: figures, desc: reconall, suffix: T1w} + caption: Surfaces (white and pial) reconstructed with FreeSurfer (<code>recon-all</code>) + overlaid on the participant's T1w template. + subtitle: Surface reconstruction +- name: About + reportlets: + - bids: {datatype: figures, desc: about, suffix: T1w} + - custom: boilerplate + path: '{reportlets_dir}/logs' Yes, the CITATION.html is written inside logs. I had the debug that line to make it work, so I'm sure about this. — Reply to this email directly, view it on GitHub <#3191 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAESDRRGJFB7JCALAMQNZ5LYRD5OHAVCNFSM6AAAAABA6ZGRK6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJRGM4DMNRRGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Co-authored-by: Oscar Esteban <code@oscaresteban.es>

effigies

Looks like nireports uses out_dir, not output_dir:

https://github.com/nipreps/nireports/blob/0d893ff5cbe75961b34227cffc54dab219fa4a94/nireports/assembler/report.py#L285-L289

fmriprep/data/reports-spec-anat.yml

fmriprep/data/reports-spec.yml

fmriprep/cli/run.py

fmriprep/cli/workflow.py

effigies

Outputs look good to me.

I pushed a few additional changes.

I emptied out the SVG files to avoid cluttering the repository. We don't care about these contents, so we don't need git to track these. We'll need to squash to prevent the commits that do contain these from staying in the repository long-term.
In general, we want to avoid using __file__, as that puts constraints on installers. In some cases, an installer may want to keep the package zipped, and __file__ is not zip-safe. The fmriprep.data.load object lets us have Path-like objects when we need to read things and actual Paths when we need files that definitely exist on the filesystem. See https://www.nipreps.org/niworkflows/master/api/niworkflows.data.html for the API.
I used a tmp_path fixture to allow us to avoid writing into the package directory and delete files at the end.

LMK what you think, but I think this should be good to merge if the tests pass.

effigies · 2024-01-31T20:25:53Z

Going to go ahead and get this in, re-update the style PR, and we can address any concerns with my last-second hacks in a separate PR.

Thanks for all your effort on this!

celprov · 2024-02-01T09:28:32Z

Thank you @effigies for your help finalizing it! I'm happy this one is out of the door :)

…ly sampled dataset (nipreps#3191) enh: beyond a certain number of session for a single subject (4), we separate the anatomical report from the functional and we separate the functional reports per session. For fewer sessions than 4, we keep the original behavior of aggregating all sessions and the anatomical in one report. enh: find the suitable bootstrapfile within generate_reports rather than before enh: add session_list argument to generate_reports Builds on top of nipreps#3177 Closes nipreps#3080 --------- Co-authored-by: Oscar Esteban <code@oscaresteban.es> Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>

24.0.0 (June 17, 2024) New feature release in the 24.0.x series. This release is an incremental improvement on 23.2.x, with some fixes for bugs discovered in the updated workflow. New features include separation of HTML reports by session for subjects with many BOLD runs, a new ``--fs-no-resume`` option to improve interoperability with less typical FreeSurfer directories, such as those generated by longitudinal FreeSurfer or FastSurfer, and adoption of DatasetLinks and BIDS-URIs, to follow the recommendations of recent versions of BIDS. With thanks to Dimitri Papadopoulos, Basile Pinsard, Celine Provins, Taylor Salo and Wang Hao-Ting for their contributions! * FIX: Add "double" type to allowed DisplacementFieldTransform (#3287) * FIX: Require recent templateflow, select correct aparc dseg.tsv (#3256) * FIX: Ensure proper templates are retrieved with sloppy (#3251) * FIX: Delete summary from functional report when separated by sessions (#3223) * FIX: Support lists in bids filter file containing ``null`` or ``*`` (#3215) * FIX: Re-enable anat fasttrack for dataset without t1w (#3202) * ENH: Use BIDSURI in init_ds_boldmask_wf (#3297) * ENH: Add templateflow to DatasetLinks (#3267) * ENH: Track proximal sources of functional GIFTIs (#3263) * ENH: Support named derivative paths (#3264) * ENH: Track Sources for standard-space outputs (#3262) * ENH: Add --fs-no-resume option to reuse existing FreeSurfer outputs without resuming (#3142) * ENH: Use BIDS URIs to track Sources in sidecars (#3255) * ENH: Ignore unselected subjects in BIDSLayoutIndexer (#3236) * ENH: Add metadata for motion parameters (#3245) * ENH: Separate anatomical and functional reports per session for densely sampled dataset (#3191) * ENH: Leverage T2w if available for BOLD -> anat coregistration (#3208) * RF: Fix ITK warp conversion to nitransforms format (#3300) * RF: Load report assembler from nireports (#3177) * DOC: Clarify ``--dvars-spike-threshold`` uses standardized DVARS (#3205) * TST: Update test to reflect new report generation behavior (#3210) * STY: Manual conversions to f-strings (#3241) * STY: Apply ruff/pyupgrade rule UP031 (#3280) * STY: Lint and style check full repository (#3221) * STY: Adopt ruff for linting and formatting (#3206) * MNT: Pin libitk 5.3 and note dependencies (#3298) * MNT: Upgrade ruff pre-commit, add fixing checks (#3283) * MNT: Complete transition from flake8/black to ruff (#3279) * MNT: Apply Repo-Review suggestions (#3194) * MNT: Verbatim copy of Apache license 2.0 (#3259) * MNT: Bump cryptography from 41.0.7 to 42.0.4 (#3234) * MNT: Drop copyright year, unused dunder fields (#3247) * MNT: Update environment pins (#3226) * MNT: Bump codecov/codecov-action from 3 to 4 (#3219) * DOCKER: Restore mincinfo binary (#3249) * CI: Move to new circle machine tags (#3248) * CI: Avoid ruff warning (#3244) * CI: Pass ruff tests (#3243)

celprov added 7 commits December 15, 2023 10:43

fix: remove deprecated packagename argument of generate_reports

0e36362

enh: beyond a certain number of sessions per subject, we separate the…

78d15a6

… anatomical report and the functional reports per session. fix: let generate_reports() define the default config file

fix: desambiguate config module and config file by renaming the file

3e557cd

fix: fix functional report generation

70bbf12

fix: generate reports only for the session indicated in the bids_filters

f63b5e8

fix: session_list needs to be a list

7ff8007

fix: remove ses- prefixes

ccc60b4

celprov mentioned this pull request Jan 15, 2024

Running fMRIPrep roadmap TheAxonLab/hcph-sops#417

Open

3 tasks

celprov added 2 commits January 17, 2024 10:59

Merge remote-tracking branch 'origin/maint/load-report-assembler-from…

fc52e20

…-nireports' into enh/sep_anat_func_report

Merge remote-tracking branch 'upstream/master' into enh/sep_anat_func…

e802a75

…_report

celprov force-pushed the enh/sep_anat_func_report branch from 2d35de6 to e802a75 Compare January 18, 2024 16:38

Merge branch 'master' into enh/sep_anat_func_report

3ea5c77

oesteban reviewed Jan 19, 2024

View reviewed changes

fmriprep/reports/core.py Outdated Show resolved Hide resolved

celprov added 2 commits January 19, 2024 15:06

fix: transform the maximum of aggregated sessions in one subject's re…

87db1c9

…port as a cli option

Merge branch 'enh/sep_anat_func_report' of github.com:celprov/fmripre…

f595064

…p into enh/sep_anat_func_report

effigies reviewed Jan 26, 2024

View reviewed changes

celprov added 5 commits January 29, 2024 11:07

fix: strip sub- prefix in the html name of the QC-FC reports

9eed4fd

fix: turn the subject into a list if it is inputted as a string

fix: write separate function to call report generation

13c3a54

fix: apply Chris' review comments

c87e45c

fix: rename max-ses-agr to aggregated-session-reports

4e87c48

celprov force-pushed the enh/sep_anat_func_report branch from 0de8074 to 4e87c48 Compare January 29, 2024 16:23

celprov added 2 commits January 30, 2024 13:52

enh: add test to check that the error message has been incorporated i…

3249c18

…n the report. enh: add test to check that the boilerplate has been incorporated in the reports. fix: condense all the tests in one implementation

celprov force-pushed the enh/sep_anat_func_report branch 3 times, most recently from 4182c9e to 3249c18 Compare January 30, 2024 13:34

sty: styling to PEP8 and isort

857011d

celprov requested review from effigies and oesteban January 30, 2024 13:47

oesteban reviewed Jan 30, 2024

View reviewed changes

Apply suggestions from code review

edd3d8e

Co-authored-by: Oscar Esteban <code@oscaresteban.es>

celprov force-pushed the enh/sep_anat_func_report branch from 1c56024 to edd3d8e Compare January 30, 2024 14:13

effigies reviewed Jan 30, 2024

View reviewed changes

fmriprep/data/reports-spec-anat.yml Outdated Show resolved Hide resolved

fmriprep/data/reports-spec.yml Outdated Show resolved Hide resolved

Fix template key names

0f6b436

effigies reviewed Jan 31, 2024

View reviewed changes

fmriprep/cli/run.py Outdated Show resolved Hide resolved

FIX: Get bids_filters safely

81bc459

effigies reviewed Jan 31, 2024

View reviewed changes

fmriprep/cli/workflow.py Outdated Show resolved Hide resolved

effigies added 4 commits January 30, 2024 21:22

FIX: Get bids_filters safely (again)

cf31987

MNT: Strip SVG contents - we will squash to prune large objects

763acde

RF: Use data loader instead of __file__

6756f45

TEST: Use tmp_path to avoid writing files into package directory

f846469

effigies approved these changes Jan 31, 2024

View reviewed changes

TEST: Skip test in installed runs

c1a3eac

effigies force-pushed the enh/sep_anat_func_report branch from a5e5ed5 to d0b7289 Compare January 31, 2024 18:49

STY: black

e89765d

effigies force-pushed the enh/sep_anat_func_report branch from d0b7289 to e89765d Compare January 31, 2024 18:52

Merge branch 'master' into enh/sep_anat_func_report

7884e58

effigies merged commit f3e95d9 into nipreps:master Jan 31, 2024
17 checks passed

tsalo mentioned this pull request Jan 31, 2024

Split reports by session for longitudinal datasets PennLINC/aslprep#390

Open

oesteban mentioned this pull request Feb 1, 2024

Issues with reports after migrating to NiReports #3211

Open

3 tasks

celprov mentioned this pull request May 13, 2024

Despite many sessions for a single subject, the reports did not get separated per session #3285

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: separate anatomical and functional reports per session for densely sampled dataset #3191

ENH: separate anatomical and functional reports per session for densely sampled dataset #3191

celprov commented Dec 21, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading

celprov commented Jan 19, 2024

celprov commented Jan 30, 2024 •

edited

Loading

oesteban left a comment

oesteban commented Jan 30, 2024 via email

effigies left a comment

effigies left a comment

effigies commented Jan 31, 2024

celprov commented Feb 1, 2024

ENH: separate anatomical and functional reports per session for densely sampled dataset #3191

ENH: separate anatomical and functional reports per session for densely sampled dataset #3191

Conversation

celprov commented Dec 21, 2023 • edited Loading

codecov bot commented Dec 21, 2023 • edited Loading

Codecov Report

celprov commented Jan 19, 2024

celprov commented Jan 30, 2024 • edited Loading

oesteban left a comment

Choose a reason for hiding this comment

oesteban commented Jan 30, 2024 via email

effigies left a comment

Choose a reason for hiding this comment

effigies left a comment

Choose a reason for hiding this comment

effigies commented Jan 31, 2024

celprov commented Feb 1, 2024

celprov commented Dec 21, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading

celprov commented Jan 30, 2024 •

edited

Loading