Skip to content

Conversation

@sappelhoff
Copy link
Member

The glossary entry was a bit misleading due to its brevity:

  1. there is no limit to fiducial points (i.e., there can be more than 3)
  2. LPA, RPA, and NAS are anatomical landmarks, but "fiducials" is often used as a synonymous term for that

Potential links to add:

@sappelhoff
Copy link
Member Author

Also, this example part

check_ch = ['Oz', 'Fpz', 'T7', 'T8']
ch_idx = [fake_evoked.ch_names.index(ch) for ch in check_ch]
pos = np.stack([fake_evoked.info['chs'][idx]['loc'][:3] for idx in ch_idx])

should probably be replaced by using this function, right?

def get_positions(self):
"""Get all channel and fiducial positions.
Returns
-------
positions : dict
A dictionary of the positions for channels (``ch_pos``),
coordinate frame (``coord_frame``), nasion (``nasion``),
left preauricular point (``lpa``),
right preauricular point (``rpa``),
Head Shape Polhemus (``hsp``), and
Head Position Indicator(``hpi``).
E.g.::
{
'ch_pos': {'EEG061': [0, 0, 0]},
'nasion': [0, 0, 1],
'lpa': [0, 1, 0],
'rpa': [1, 0, 0],
'hsp': None,
'hpi': None
}
"""
# get channel positions as dict
ch_pos = self._get_ch_pos()
# _get_fid_coords(self.dig)
# get coordframe and fiducial coordinates
montage_bunch = _get_data_as_dict_from_dig(self.dig)
coord_frame = _frame_to_str.get(montage_bunch.coord_frame)
# return dictionary
positions = dict(
ch_pos=ch_pos,
coord_frame=coord_frame,
nasion=montage_bunch.nasion,
lpa=montage_bunch.lpa,
rpa=montage_bunch.rpa,
hsp=montage_bunch.hsp,
hpi=montage_bunch.hpi,
)
return positions

@sappelhoff
Copy link
Member Author

test failure seems to be due to timeout:

=========================== short test summary info ============================
FAILED mne/commands/tests/test_commands.py::test_watershed_bem - Failed: Time...
FAILED mne/commands/tests/test_commands.py::test_flash_bem - Failed: Timeout ...
=============== 2 failed, 3059 deselected in 172

https://dev.azure.com/mne-tools/mne-python/_build/results?buildId=11095&view=logs&j=25a52bd8-0e32-5080-16fd-ef6520ac93ab&t=ed0c6a43-6c02-5c37-64bf-ac1c672f0690

Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

just 2 suggestions, neither are blockers.

Co-authored-by: Daniel McCloy <dan@mccloy.info>
@drammock drammock merged commit a46546b into mne-tools:master Jan 18, 2021
@drammock
Copy link
Member

thanks @sappelhoff !

@sappelhoff sappelhoff deleted the fid branch January 18, 2021 17:42
larsoner added a commit to vpeterson/mne-python that referenced this pull request Feb 25, 2021
* upstream/master: (66 commits)
  MRG, ENH: Add infant template downloader (mne-tools#8738)
  ENH: add reader for NeuroElectrics .nedf files (mne-tools#8734)
  DOC: improve glossary entry about fiducials (mne-tools#8763)
  MRG, ENH: Add Report.add_custom_css (mne-tools#8762)
  BUG, DOC: read_raw_egi didn't support pathlib.Path; update read_raw() docstring (mne-tools#8759)
  Add "dbs" as new channel type (mne-tools#8739)
  MRG, VIZ: Fix title position in plot_sensors (mne-tools#8752)
  MRG: Support for non-FIFF files in Report.parse_folder (mne-tools#8744)
  MRG, VIZ, FIX: sEEG picking in _prepare_topomap_plot() (mne-tools#8736)
  DOC: don't use single letter variable name in _compute_forward (mne-tools#8727)
  WIP: Fix search [skip github] [skip azp] (mne-tools#8742)
  WIP: Compare Beer-lambert to HOMER (mne-tools#8711)
  MRG: bump spyder version (mne-tools#8020)
  FIX anon with IO round trip (mne-tools#8731)
  fix set_bipolar_reference for Epochs (mne-tools#8728)
  WIP: Add width argument, reduce default (mne-tools#8725)
  ENH: Add toggle-all button to Report (mne-tools#8723)
  fix int/float conversion in nicolet header (mne-tools#8712)
  MRG, BUG: Fix Report.add_bem_to_section n_jobs != 1 (mne-tools#8713)
  MRG, DOC: Make "rank" options in docs more accessible (mne-tools#8707)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants