Skip to content

Commit

Permalink
BUG: Fix bug with reading dig strings (#13083)
Browse files Browse the repository at this point in the history
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
larsoner and autofix-ci[bot] authored Feb 12, 2025
1 parent cf5ef5f commit 77195a2
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 31 deletions.
1 change: 1 addition & 0 deletions doc/changes/devel/13083.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug with reading digitization points from digitization strings with newer MEGIN systems, by `Eric Larson`_.
3 changes: 3 additions & 0 deletions mne/_fiff/_digitization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
26 changes: 4 additions & 22 deletions mne/_fiff/meas_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -1970,7 +1970,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
Expand All @@ -1990,26 +1990,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
Expand Down
2 changes: 1 addition & 1 deletion mne/_fiff/open.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 17 additions & 7 deletions mne/_fiff/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,19 +270,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):
Expand Down Expand Up @@ -378,18 +385,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_",
Expand Down
11 changes: 10 additions & 1 deletion mne/_fiff/tests/test_meas_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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.

0 comments on commit 77195a2

Please sign in to comment.