Merge remote-tracking branch 'upstream/maint/1.1.x'

Author: effigies

CHANGES.rst

@@ -1,3 +1,14 @@
+1.1.8 (February 26, 2020)
+=========================
+Bug-fix release in the 1.1.x series.
+
+This release includes some minor improvements to formatting of reports and derivative metadata.
+
+* FIX: Check fo valid qform before calculating change (#466) @effigies
+* ENH: Display errors as summary/details elements (#464) @effigies
+* ENH: Add a pure-Python ApplyMask interface, based on NiBabel (#463) @oesteban
+* RF: Replace ``os`` operations with ``pathlib``, indent JSON sidecars (#467) @mgxd
+
 1.1.7 (February 14, 2020)
 =========================
 Minor improvements to enable fMRIPrep 20.0.0 release.

niworkflows/interfaces/cifti.py

@@ -1,8 +1,7 @@
 # emacs: -*- mode: python; py-indent-offset: 4; indent-tabs-mode: nil -*-
 # vi: set ft=python sts=4 ts=4 sw=4 et:
 """Handling connectivity: combines FreeSurfer surfaces with subcortical volumes."""
-import os
-from glob import glob
+from pathlib import Path
 import json
 import warnings
 
@@ -209,12 +208,7 @@ def _get_cifti_data(surface, volume, subjects_dir=None, density=None):
     # fMRIPrep grayordinates
     if volume == "MNI152NLin2009cAsym":
         tpl_kwargs.update({'resolution': '2', 'desc': 'DKT31'})
-        annotation_files = sorted(glob(os.path.join(
-            subjects_dir,
-            surface,
-            'label',
-            '*h.aparc.annot'
-        )))
+        annotation_files = sorted((subjects_dir / surface / 'label').glob('*h.aparc.annot'))
     # HCP grayordinates
     elif volume == 'MNI152NLin6Asym':
         # templateflow specific resolutions (2mm, 1.6mm)
@@ -253,7 +247,7 @@ def _get_cifti_variant(surface, volume, density=None):
     Examples
     --------
     >>> metafile, variant, _ = _get_cifti_variant('fsaverage5', 'MNI152NLin2009cAsym')
-    >>> metafile  # doctest: +ELLIPSIS
+    >>> str(metafile)  # doctest: +ELLIPSIS
     '.../dtseries_variant.json'
     >>> variant
     'fMRIPrep grayordinates'
@@ -279,7 +273,7 @@ def _get_cifti_variant(surface, volume, density=None):
         )
 
     grayords = None
-    out_metadata = os.path.abspath('dtseries_variant.json')
+    out_metadata = Path.cwd() / 'dtseries_variant.json'
     out_json = {
         'space': variant,
         'surface': surface,
@@ -290,8 +284,7 @@ def _get_cifti_variant(surface, volume, density=None):
         grayords = {'32k': '91k', '59k': '170k'}[density]
         out_json['grayordinates'] = grayords
 
-    with open(out_metadata, 'w') as fp:
-        json.dump(out_json, fp)
+    out_metadata.write_text(json.dumps(out_json, indent=2))
     return out_metadata, variant, grayords
 
 
@@ -428,4 +421,4 @@ def _create_cifti_image(bold_file, label_file, bold_surfs, annotation_files, tr,
 
     out_file = "{}.dtseries.nii".format(split_filename(bold_file)[1])
     ci.save(img, out_file)
-    return os.path.join(os.getcwd(), out_file)
+    return Path.cwd() / out_file

niworkflows/interfaces/images.py

@@ -420,25 +420,28 @@ def _run_interface(self, runtime):
         elif (valid_sform and sform_code > 0) and (not matching_affines or qform_code == 0):
             img.set_qform(sform, sform_code)
             new_qform = img.get_qform()
-            if np.allclose(new_qform, qform) and qform_code > 0:
-                # False alarm
-                self._results['out_file'] = self.inputs.in_file
-                open(out_report, 'w').close()
-                self._results['out_report'] = out_report
-                return runtime
-            diff = np.linalg.inv(qform) @ new_qform
-            trans, rot, _, _ = transforms3d.affines.decompose44(diff)
-            angle = transforms3d.axangles.mat2axangle(rot)[1]
-            total_trans = np.sqrt(np.sum(trans * trans))  # Add angle and total_trans to report
-            warning_txt = 'Note on orientation: qform matrix overwritten'
-            description = """\
+            if valid_qform:
+                # False alarm - the difference is due to precision loss of qform
+                if np.allclose(new_qform, qform) and qform_code > 0:
+                    self._results['out_file'] = self.inputs.in_file
+                    open(out_report, 'w').close()
+                    self._results['out_report'] = out_report
+                    return runtime
+                # Replacing an existing, valid qform. Report magnitude of change.
+                diff = np.linalg.inv(qform) @ new_qform
+                trans, rot, _, _ = transforms3d.affines.decompose44(diff)
+                angle = transforms3d.axangles.mat2axangle(rot)[1]
+                total_trans = np.sqrt(np.sum(trans * trans))  # Add angle and total_trans to report
+                warning_txt = 'Note on orientation: qform matrix overwritten'
+                description = """\
     <p class="elem-desc">
     The qform has been copied from sform.
     The difference in angle is {angle:.02g}.
     The difference in translation is {total_trans:.02g}.
     </p>
     """.format(angle=angle, total_trans=total_trans)
-            if not valid_qform and qform_code > 0:
+            elif qform_code > 0:
+                # qform code indicates the qform is supposed to be valid. Use more stridency.
                 warning_txt = 'WARNING - Invalid qform information'
                 description = """\
 <p class="elem-desc">
@@ -448,6 +451,10 @@ def _run_interface(self, runtime):
     by the scanner is advised.
 </p>
 """
+            else:  # qform_code == 0
+                # qform is not expected to be valids. Simple note.
+                warning_txt = 'Note on orientation: qform matrix overwritten'
+                description = '<p class="elem-desc">The qform has been copied from sform.</p>'
         # Rows 5-6:
         else:
             affine = img.header.get_base_affine()

niworkflows/interfaces/tests/test_images.py

@@ -34,11 +34,51 @@ def test_qformsform_warning(tmp_path, qform_add, sform_add, expectation):
     validate = pe.Node(im.ValidateImage(), name='validate', base_dir=str(tmp_path))
     validate.inputs.in_file = fname
     res = validate.run()
+    out_report = Path(res.outputs.out_report).read_text()
     if expectation == 'warn':
-        assert "Note on" in Path(res.outputs.out_report).read_text()
-        assert len(Path(res.outputs.out_report).read_text()) > 0
+        assert "Note on" in out_report
     elif expectation == 'no_warn':
-        assert len(Path(res.outputs.out_report).read_text()) == 0
+        assert len(out_report) == 0
+
+
+@pytest.mark.parametrize('qform_code, warning_text', [
+    (0, "Note on orientation"),
+    (1, "WARNING - Invalid qform"),
+])
+def test_bad_qform(tmp_path, qform_code, warning_text):
+    fname = str(tmp_path / 'test.nii')
+
+    # make a random image
+    random_data = np.random.random(size=(5, 5, 5) + (5,))
+    img = nb.Nifti1Image(random_data, np.eye(4))
+
+    # Some magic terms from a bad qform in the wild
+    img.header['qform_code'] = qform_code
+    img.header['quatern_b'] = 0
+    img.header['quatern_c'] = 0.998322
+    img.header['quatern_d'] = -0.0579125
+    img.to_filename(fname)
+
+    validate = pe.Node(im.ValidateImage(), name='validate', base_dir=str(tmp_path))
+    validate.inputs.in_file = fname
+    res = validate.run()
+    assert warning_text in Path(res.outputs.out_report).read_text()
+
+
+def test_no_good_affines(tmp_path):
+    fname = str(tmp_path / 'test.nii')
+
+    # make a random image
+    random_data = np.random.random(size=(5, 5, 5) + (5,))
+    img = nb.Nifti1Image(random_data, None)
+    img.header['qform_code'] = 0
+    img.header['sform_code'] = 0
+    img.to_filename(fname)
+
+    validate = pe.Node(im.ValidateImage(), name='validate', base_dir=str(tmp_path))
+    validate.inputs.in_file = fname
+    res = validate.run()
+    assert 'WARNING - Missing orientation information' in Path(res.outputs.out_report).read_text()
 
 
 @pytest.mark.parametrize('nvols, nmasks, ext, factor', [

niworkflows/reports/core.py

@@ -234,7 +234,7 @@ class Report(object):
     >>> robj.generate_report()
     0
     >>> len((testdir / 'out' / 'fmriprep' / 'sub-01.html').read_text())
-    36425
+    36450
 
     .. testcleanup::
 

niworkflows/reports/report.tpl

@@ -49,6 +49,9 @@ div#boilerplate pre {
     background-color: #F8F9FA;
 }
 
+#errors div, #errors p {
+    padding-left: 1em;
+}
 </style>
 </head>
 <body>
@@ -127,12 +130,10 @@ div#boilerplate pre {
 
 <div id="errors">
     <h1 class="sub-report-title">Errors</h1>
-    <ul>
     {% for error in errors %}
-        <li>
-        <div class="nipype_error">
-            Node Name: <a target="_self" onclick="toggle('{{error.file|replace('.', '')}}_details_id');">{{ error.node }}</a><br>
-            <div id="{{error.file|replace('.', '')}}_details_id" style="display:none">
+    <details>
+        <summary>Node Name: {{ error.node }}</summary><br>
+        <div>
             File: <code>{{ error.file }}</code><br>
             Working Directory: <code>{{ error.node_dir }}</code><br>
             Inputs: <br>
@@ -142,13 +143,11 @@ div#boilerplate pre {
             {% endfor %}
             </ul>
             <pre>{{ error.traceback }}</pre>
-            </div>
         </div>
-        </li>
+    </details>
     {% else %}
-        <li>No errors to report!</li>
+    <p>No errors to report!</p>
     {% endfor %}
-    </ul>
 </div>