[RTM] ENH: Subject-level summary report #647
Conversation
|
(Also, wanted to push it before rebasing/merging master, after Oscar's recent PRs.) |
effigies
force-pushed
the
report/summary
branch
from
1934728
to
4b21b13
Compare
effigies
force-pushed
the
report/summary
branch
from
4b21b13
to
da316d6
Compare
effigies
changed the title
| @@ -2,14 +2,20 @@ | |||
| "sub_reports": | |||
| [ | |||
| { | |||
| "name": "Anatomical", | |||
| "name": "Summary", | |||
| "elements": | |||
| [ | |||
| { | |||
| "name": "anat/summary", | |||
| "file_pattern": "anat/.*_summary", | |||
There was a problem hiding this comment.
DerivativesDataSink makes some naming assumptions that don't make it automatic to add a new top-level namespace. I'm not sure that it's worth it, for a single-element entry.
There was a problem hiding this comment.
I think we can live with this :).
Additionally, we want #639 to be closed as soon as possible, and modifying DerivativesDataSink (which is currently working) may break it.
There was a problem hiding this comment.
Yeah. This comment turned into #649. No plans to change it in this PR.
fmriprep/workflows/base.py
Outdated
| (bidssrc, bids_info, [(('t1w', fix_multi_T1w_source_name), 'in_file')]), | ||
| (bidssrc, summary, [('t1w', 't1w'), | ||
| ('t2w', 't2w'), | ||
| ('bold', 'bold')]), |
There was a problem hiding this comment.
This currently provides the name of all BOLD series, rather than just the ones in the current run.
This makes this node more stable when running different task IDs in different processes, but it does mean that if someone doesn't choose to run FMRIPREP on all of their data, then the summary may not match the rest of the outputs.
To make it show just the images in the current execution, we'd just set summary.inputs.bold = subject_data['bold']
| # Date not included - update timestamp only if version or command changes | ||
|
|
||
|
|
||
| class AboutSummary(SummaryInterface): |
There was a problem hiding this comment.
This interface captures the most recent version/command line used. The timestamp is only updated if either changes. Seem reasonable @chrisfilo?
|
@oesteban @chrisfilo Any final comments? |
fmriprep/interfaces/reports.py
Outdated
| subjects_dir = Directory(desc='FreeSurfer subjects directory') | ||
| subject_id = traits.Str(desc='FreeSurfer subject ID') | ||
| subject_id = traits.Str(desc='Subject ID') |
There was a problem hiding this comment.
Use Str from nipype.interfaces.base better than traits.Str? I think we try to shadow traits.Str but it is safer to use the earlier (and it is already imported in this file...)
|
Fixed. I'll merge on passing tests. |
This one's taking a bit more work than I expected. The reportlet is in place, but I'm not sure what the best way to integrate it into reports would be, since it will end up at the same level as "Anatomical" and "Functional", when all of the other reportlets are going under subheadings.
I've started fiddling with the template, but figured I'd put up what I've got at the moment.
Closes #639.