Skip to content
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.

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

[FIX] Add CE agent to output figure filename templates #482

Merged
merged 9 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
15 changes: 6 additions & 9 deletions niworkflows/reports/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class Report(object):
>>> robj.generate_report()
0
>>> len((testdir / 'out' / 'fmriprep' / 'sub-01.html').read_text())
36450
36540

.. testcleanup::

Expand Down Expand Up @@ -311,17 +311,14 @@ def index(self, config):
reportlets = []
for c in list_combos:
# do not display entities with the value None.
c = list(filter(None, c))
ent = list(compress(entities, c))
missing_entities = list(set(entities) - set(ent))
c_filt = list(filter(None, c))
ent_filt = list(compress(entities, c_filt))
# Set a common title for this particular combination c
title = 'Reports for: %s.' % ', '.join(
['%s <span class="bids-entity">%s</span>' % (ent[i], c[i])
for i in range(len(c))])
['%s <span class="bids-entity">%s</span>' % (ent_filt[i], c_filt[i])
for i in range(len(c_filt))])
for cfg in subrep_cfg['reportlets']:
for m_e in missing_entities:
cfg['bids'].pop(m_e, None)
cfg['bids'].update({ent[i]: c[i] for i in range(len(c))})
cfg['bids'].update({entities[i]: c[i] for i in range(len(c))})
rlet = Reportlet(self.layout, self.out_dir, config=cfg)
if not rlet.is_empty():
rlet.title = title
Expand Down
4 changes: 2 additions & 2 deletions niworkflows/reports/figures.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
"pattern": "[_/\\\\]acq-([a-zA-Z0-9]+)"
},
{
"name": "ce",
"name": "ceagent",
Copy link
Member

Choose a reason for hiding this comment

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

Does this have any API effect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I do not believe so. This does not impact the recently added --bids-filter, and makes figures.json naming consistent with fmriprep.yml.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. @oesteban Is this consistent with your understanding as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I also think this should not affect the API

"pattern": "[_/\\\\]ce-([a-zA-Z0-9]+)"
},
{
"name": "reconstruction",
"pattern": "[_/\\\\]rec-([a-zA-Z0-9]+)"
},
{
"name": "dir",
"name": "direction",
"pattern": "[_/\\\\]dir-([a-zA-Z0-9]+)"
},
{
Expand Down
21 changes: 12 additions & 9 deletions niworkflows/reports/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,28 @@ def bids_sessions(tmpdir_factory):
pattern = (
"sub-{subject}[/ses-{session}]/{datatype<anat|func>}/"
"sub-{subject}[_ses-{session}][_task-{task}][_acq-{acquisition}]"
"[_ce-{contrast}][_dir-{direction}][_rec-{reconstruction}]"
"[_ce-{ceagent}][_dir-{direction}][_rec-{reconstruction}]"
"[_mod-{modality}][_run-{run}][_echo-{echo}][_space-{space}]"
"[_desc-{desc}]_{suffix<dseg|T1w|bold>}.{extension<svg>}"
)
subjects = ['01']
tasks = ['t1', 't2', 't3']
runs = ['01', '02', None]
ces = ['none', 'Gd']
descs = ['aroma', 'bbregister', 'carpetplot', 'rois']
# create functional data for both sessions
ses1_combos = product(subjects, ['1'], tasks, runs, descs)
ses2_combos = product(subjects, ['2'], tasks, [None], descs)
ses1_combos = product(subjects, ['1'], tasks, [None], runs, descs)
ses2_combos = product(subjects, ['2'], tasks, ces, [None], descs)
# have no runs in the second session (ex: dmriprep test data)
# https://github.com/nipreps/dmriprep/pull/59
all_combos = list(ses1_combos) + list(ses2_combos)

for subject, session, task, run, desc in all_combos:
for subject, session, task, ce, run, desc in all_combos:
entities = {
'subject': subject,
'session': session,
'task': task,
'ceagent': ce,
'run': run,
'desc': desc,
'extension': 'svg',
Expand Down Expand Up @@ -143,10 +145,10 @@ def test_process_orderings_small(test_report1, orderings,
@pytest.mark.parametrize(
"orderings,expected_entities,first_value_combo,last_value_combo",
[
(['session', 'task', 'run'],
['session', 'task', 'run'],
('1', 't1', None),
('2', 't3', None),
(['session', 'task', 'ceagent', 'run'],
['session', 'task', 'ceagent', 'run'],
('1', 't1', None, None),
('2', 't3', 'none', None),
),
(['run', 'task', 'session'],
['run', 'task', 'session'],
Expand Down Expand Up @@ -182,6 +184,7 @@ def test_process_orderings_large(test_report2, orderings,
("run"),
("session,task"),
("session,task,run"),
("session,task,ceagent,run"),
("session,task,acquisition,ceagent,reconstruction,direction,run,echo"),
("session,task,run,madeupentity"),
])
Expand All @@ -198,7 +201,7 @@ def test_generated_reportlets(bids_sessions, ordering):
# expected number of reportlets
expected_reportlets_num = len(report.layout.get(extension='svg'))
# bids_session uses these entities
needed_entities = ['session', 'task', 'run']
needed_entities = ['session', 'task', 'ceagent', 'run']
# the last section is the most recently run
reportlets_num = len(report.sections[-1].reportlets)
# if ordering does not contain all the relevent entities
Expand Down