Backport PR #13083 on branch maint/1.9 (BUG: Fix bug with reading dig strings) (#13114)

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Author: meeseeksmachine

doc/changes/devel/13083.bugfix.rst

@@ -0,0 +1 @@
+Fix bug with reading digitization points from digitization strings with newer MEGIN systems, by `Eric Larson`_.

mne/_fiff/_digitization.py

@@ -181,6 +181,9 @@ def _read_dig_fif(fid, meas_info, *, return_ch_names=False):
             if kind == FIFF.FIFF_DIG_POINT:
                 tag = read_tag(fid, pos)
                 dig.append(tag.data)
+            elif kind == FIFF.FIFF_DIG_STRING:
+                tag = read_tag(fid, pos)
+                dig.extend(tag.data)
             elif kind == FIFF.FIFF_MNE_COORD_FRAME:
                 tag = read_tag(fid, pos)
                 coord_frame = _coord_frame_named.get(int(tag.data.item()))

mne/_fiff/meas_info.py

@@ -46,7 +46,7 @@
     write_dig,
 )
 from .compensator import get_current_comp
-from .constants import FIFF, _ch_unit_mul_named, _coord_frame_named
+from .constants import FIFF, _ch_unit_mul_named
 from .ctf_comp import _read_ctf_comp, write_ctf_comp
 from .open import fiff_open
 from .pick import (
@@ -1961,7 +1961,7 @@ def _simplify_info(info, *, keep=()):
 
 
 @verbose
-def read_fiducials(fname, verbose=None):
+def read_fiducials(fname, *, verbose=None):
     """Read fiducials from a fiff file.
 
     Parameters
@@ -1981,26 +1981,8 @@ def read_fiducials(fname, verbose=None):
     fname = _check_fname(fname=fname, overwrite="read", must_exist=True)
     fid, tree, _ = fiff_open(fname)
     with fid:
-        isotrak = dir_tree_find(tree, FIFF.FIFFB_ISOTRAK)
-        isotrak = isotrak[0]
-        pts = []
-        coord_frame = FIFF.FIFFV_COORD_HEAD
-        for k in range(isotrak["nent"]):
-            kind = isotrak["directory"][k].kind
-            pos = isotrak["directory"][k].pos
-            if kind == FIFF.FIFF_DIG_POINT:
-                tag = read_tag(fid, pos)
-                pts.append(DigPoint(tag.data))
-            elif kind == FIFF.FIFF_MNE_COORD_FRAME:
-                tag = read_tag(fid, pos)
-                coord_frame = tag.data[0]
-                coord_frame = _coord_frame_named.get(coord_frame, coord_frame)
-
-    # coord_frame is not stored in the tag
-    for pt in pts:
-        pt["coord_frame"] = coord_frame
-
-    return pts, coord_frame
+        pts = _read_dig_fif(fid, tree)
+    return pts, pts[0]["coord_frame"]
 
 
 @verbose

mne/_fiff/open.py

@@ -348,7 +348,7 @@ def _show_tree(
                         postpend += " ... list len=" + str(len(tag.data))
                     elif issparse(tag.data):
                         postpend += (
-                            f" ... sparse ({tag.data.getformat()}) shape="
+                            f" ... sparse ({tag.data.__class__.__name__}) shape="
                             f"{tag.data.shape}"
                         )
                     else:

mne/_fiff/tag.py

@@ -271,19 +271,26 @@ def _read_id_struct(fid, tag, shape, rlims):
     )
 
 
-def _read_dig_point_struct(fid, tag, shape, rlims):
+def _read_dig_point_struct(fid, tag, shape, rlims, *, string=False):
     """Read dig point struct tag."""
     kind = int(np.frombuffer(fid.read(4), dtype=">i4").item())
     kind = _dig_kind_named.get(kind, kind)
     ident = int(np.frombuffer(fid.read(4), dtype=">i4").item())
     if kind == FIFF.FIFFV_POINT_CARDINAL:
         ident = _dig_cardinal_named.get(ident, ident)
-    return dict(
-        kind=kind,
-        ident=ident,
-        r=np.frombuffer(fid.read(12), dtype=">f4"),
-        coord_frame=FIFF.FIFFV_COORD_UNKNOWN,
-    )
+    n = 1 if not string else int(np.frombuffer(fid.read(4), dtype=">i4").item())
+    out = [
+        dict(
+            kind=kind,
+            ident=ident,
+            r=np.frombuffer(fid.read(12), dtype=">f4"),
+            coord_frame=FIFF.FIFFV_COORD_UNKNOWN,
+        )
+        for _ in range(n)
+    ]
+    if not string:
+        out = out[0]
+    return out
 
 
 def _read_coord_trans_struct(fid, tag, shape, rlims):
@@ -379,18 +386,21 @@ def _read_julian(fid, tag, shape, rlims):
     FIFF.FIFFT_COMPLEX_DOUBLE: _read_complex_double,
     FIFF.FIFFT_ID_STRUCT: _read_id_struct,
     FIFF.FIFFT_DIG_POINT_STRUCT: _read_dig_point_struct,
+    FIFF.FIFFT_DIG_STRING_STRUCT: partial(_read_dig_point_struct, string=True),
     FIFF.FIFFT_COORD_TRANS_STRUCT: _read_coord_trans_struct,
     FIFF.FIFFT_CH_INFO_STRUCT: _read_ch_info_struct,
     FIFF.FIFFT_OLD_PACK: _read_old_pack,
     FIFF.FIFFT_DIR_ENTRY_STRUCT: _read_dir_entry_struct,
     FIFF.FIFFT_JULIAN: _read_julian,
+    FIFF.FIFFT_VOID: lambda fid, tag, shape, rlims: None,
 }
 _call_dict_names = {
     FIFF.FIFFT_STRING: "str",
     FIFF.FIFFT_COMPLEX_FLOAT: "c8",
     FIFF.FIFFT_COMPLEX_DOUBLE: "c16",
     FIFF.FIFFT_ID_STRUCT: "ids",
     FIFF.FIFFT_DIG_POINT_STRUCT: "dps",
+    FIFF.FIFFT_DIG_STRING_STRUCT: "dss",
     FIFF.FIFFT_COORD_TRANS_STRUCT: "cts",
     FIFF.FIFFT_CH_INFO_STRUCT: "cis",
     FIFF.FIFFT_OLD_PACK: "op_",

mne/_fiff/tests/test_meas_info.py

@@ -29,7 +29,7 @@
     write_cov,
     write_forward_solution,
 )
-from mne._fiff import meas_info
+from mne._fiff import meas_info, tag
 from mne._fiff._digitization import DigPoint, _make_dig_points
 from mne._fiff.constants import FIFF
 from mne._fiff.meas_info import (
@@ -1268,3 +1268,12 @@ def test_get_montage():
     assert len(raw.info.get_montage().ch_names) == len(ch_names)
     raw.info["bads"] = [ch_names[0]]
     assert len(raw.info.get_montage().ch_names) == len(ch_names)
+
+
+def test_tag_consistency():
+    """Test that structures for tag reading are consistent."""
+    call_set = set(tag._call_dict)
+    call_names = set(tag._call_dict_names)
+    assert call_set == call_names, "Mismatch between _call_dict and _call_dict_names"
+    # TODO: This was inspired by FIFF_DIG_STRING gh-13083, we should ideally add a test
+    # that those dig points can actually be read in correctly at some point.