MRG: Add "kind" kwarg to update_anat_landmarks() (#957)

* Add "kind" kwarg to update_anat_landmarks()

* Fixes & tests

* More tests

* Make it KeyErrors

* Update changelog

doc/whats_new.rst

@@ -40,12 +40,14 @@ Enhancements
 
 - Add support for CNT (Neuroscan) files in :func:`mne_bids.write_raw_bids`, by `Yorguin Mantilla`_ (:gh:`924`)
 
-- Add the ability to write multiple landmarks with :func:`mne_bids.write_anat` (e.g. to have separate landmarks for different sessions) by `Alexandre Gramfort`_ (:gh:`955`)
+- Add the ability to write multiple landmarks with :func:`mne_bids.write_anat` (e.g. to have separate landmarks for different sessions) via the new ``kind`` parameter, by `Alexandre Gramfort`_ (:gh:`955`)
+
+- :func:`mne_bids.get_head_mri_trans` and :func:`mne_bids.update_anat_landmarks` gained a new``kind`` parameter to specify which of multiple landmark sets to retrieve, by `Alexandre Gramfort`_ and `Richard Höchenberger`_ (:gh:`955`, :gh:`957`)
 
 API and behavior changes
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
-- ...
+- :func:`mne_bids.update_anat_landmarks` will now by default raise an exception if the requested MRI landmarks do not already exist. Use the new ``on_missing`` parameter to control this behavior, by `Richard Höchenberger`_ (:gh:`957`)
 
 Requirements
 ^^^^^^^^^^^^

mne_bids/read.py

@@ -761,7 +761,7 @@ def read_raw_bids(bids_path, extra_params=None, verbose=None):
 
 @verbose
 def get_head_mri_trans(bids_path, extra_params=None, t1_bids_path=None,
-                       fs_subject=None, fs_subjects_dir=None, *, kind="",
+                       fs_subject=None, fs_subjects_dir=None, *, kind=None,
                        verbose=None):
     """Produce transformation matrix from MEG and MRI landmark points.
 
@@ -875,7 +875,7 @@ def get_head_mri_trans(bids_path, extra_params=None, t1_bids_path=None,
     mri_coords_dict = t1w_json.get('AnatomicalLandmarkCoordinates', dict())
 
     # landmarks array: rows: [LPA, NAS, RPA]; columns: [x, y, z]
-    suffix = f"_{kind}" if kind else ""
+    suffix = f"_{kind}" if kind is not None else ""
     mri_landmarks = np.full((3, 3), np.nan)
     for landmark_name, coords in mri_coords_dict.items():
         if landmark_name.upper() == ('LPA' + suffix).upper():

mne_bids/sidecar_updates.py

@@ -10,7 +10,9 @@
 from collections import OrderedDict
 
 from mne.channels import DigMontage
-from mne.utils import logger, _validate_type, verbose
+from mne.utils import (
+    logger, _validate_type, verbose, _check_on_missing, _on_missing
+)
 from mne_bids import BIDSPath
 from mne_bids.utils import _write_json
 
@@ -138,7 +140,9 @@ def _update_sidecar(sidecar_fname, key, val):
 
 
 @verbose
-def update_anat_landmarks(bids_path, landmarks, verbose=None):
+def update_anat_landmarks(
+    bids_path, landmarks, kind=None, on_missing='raise', verbose=None
+):
     """Update the anatomical landmark coordinates of an MRI scan.
 
     This will change the ``AnatomicalLandmarkCoordinates`` entry in the
@@ -156,6 +160,22 @@ def update_anat_landmarks(bids_path, landmarks, verbose=None):
         .. note:: :func:`mne_bids.get_anat_landmarks` provides a convenient and
                   reliable way to generate the landmark coordinates in the
                   required coordinate system.
+    kind : str | None
+        The suffix of the anatomical landmark names in the JSON sidecar.
+        A suffix might be present e.g. to distinguish landmarks between
+        sessions. If provided, should not include a leading underscore ``_``.
+        For example, if the landmark names in the JSON sidecar file are
+        ``LPA_ses-1``, ``RPA_ses-1``, ``NAS_ses-1``, you should pass
+        ``'ses-1'`` here.
+        If ``None``, no suffix is appended, the landmarks named
+        ``Nasion`` (or ``NAS``), ``LPA``, and ``RPA`` will be used.
+
+        .. versionadded:: 0.10
+    on_missing : 'ignore' | 'warn' | 'raise'
+        How to behave if the specified landmarks cannot be found in the MRI
+        JSON sidecar file.
+
+        .. versionadded:: 0.10
     %(verbose)s
 
     Notes
@@ -164,6 +184,7 @@ def update_anat_landmarks(bids_path, landmarks, verbose=None):
     """
     _validate_type(item=bids_path, types=BIDSPath, item_name='bids_path')
     _validate_type(item=landmarks, types=DigMontage, item_name='landmarks')
+    _check_on_missing(on_missing)
 
     # Do some path verifications and fill in some gaps the users might have
     # left (datatype and extension)
@@ -240,12 +261,32 @@ def update_anat_landmarks(bids_path, landmarks, verbose=None):
             f'following points are missing: '
             f'{", ".join(missing_points)}')
 
-    mri_json = {
-        'AnatomicalLandmarkCoordinates': name_to_coords_map
-    }
-
     bids_path_json = bids_path.copy().update(extension='.json')
     if not bids_path_json.fpath.exists():  # Must exist before we can update it
         _write_json(bids_path_json.fpath, dict())
 
+    mri_json = json.loads(bids_path_json.fpath.read_text(encoding='utf-8'))
+    if 'AnatomicalLandmarkCoordinates' not in mri_json:
+        _on_missing(
+            on_missing=on_missing,
+            msg=f'No AnatomicalLandmarkCoordinates section found in '
+                f'{bids_path_json.fpath.name}',
+            error_klass=KeyError
+        )
+        mri_json['AnatomicalLandmarkCoordinates'] = dict()
+
+    for name, coords in name_to_coords_map.items():
+        if kind is not None:
+            name = f'{name}_{kind}'
+
+        if name not in mri_json['AnatomicalLandmarkCoordinates']:
+            _on_missing(
+                on_missing=on_missing,
+                msg=f'Anatomical landmark not found in '
+                    f'{bids_path_json.fpath.name}: {name}',
+                error_klass=KeyError
+            )
+
+        mri_json['AnatomicalLandmarkCoordinates'][name] = coords
+
     update_sidecar_json(bids_path=bids_path_json, entries=mri_json)

mne_bids/tests/test_update.py

@@ -177,12 +177,30 @@ def test_update_anat_landmarks(tmp_path):
     )
 
     # Remove JSON sidecar; updating the anatomical landmarks should re-create
-    # the file
+    # the file unless `on_missing` is `'raise'`
     bids_path_mri_json.fpath.unlink()
-    update_anat_landmarks(bids_path=bids_path_mri, landmarks=landmarks_new)
+    with pytest.raises(
+        KeyError,
+        match='No AnatomicalLandmarkCoordinates section found'
+    ):
+        update_anat_landmarks(bids_path=bids_path_mri, landmarks=landmarks_new)
+
+    update_anat_landmarks(
+        bids_path=bids_path_mri, landmarks=landmarks_new, on_missing='ignore'
+    )
 
-    with bids_path_mri_json.fpath.open(encoding='utf-8') as f:
-        mri_json = json.load(f)
+    with pytest.raises(KeyError, match='landmark not found'):
+        update_anat_landmarks(
+            bids_path=bids_path_mri, landmarks=landmarks_new, kind='ses-1'
+        )
+    update_anat_landmarks(
+        bids_path=bids_path_mri, landmarks=landmarks_new, kind='ses-1',
+        on_missing='ignore'
+    )
+
+    mri_json = json.loads(bids_path_mri_json.fpath.read_text(encoding='utf-8'))
+    assert 'NAS' in mri_json['AnatomicalLandmarkCoordinates']
+    assert 'NAS_ses-1' in mri_json['AnatomicalLandmarkCoordinates']
 
     assert np.allclose(
         landmarks_new.dig[1]['r'],