Deprecate read_montage and class Montage #6764

massich · 2019-09-13T08:58:37Z

This PR needs better description 'cos it will have a dependency

This is a list of all the digitization files I could find in my pc

        mne code
          ./mne/channels/data/montages/GSN-HydroCel-128.sfp
          ./mne/channels/data/montages/GSN-HydroCel-129.sfp
          ./mne/channels/data/montages/GSN-HydroCel-32.sfp
          ./mne/channels/data/montages/GSN-HydroCel-257.sfp
          ./mne/channels/data/montages/GSN-HydroCel-64_1.0.sfp
          ./mne/channels/data/montages/GSN-HydroCel-256.sfp
          ./mne/channels/data/montages/GSN-HydroCel-65_1.0.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-128.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-129.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-32.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-257.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-64_1.0.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-256.sfp
          ./build/lib/mne/channels/data/montages/GSN-HydroCel-65_1.0.sfp
          ./mne/channels/data/montages/mgh60.elc
          ./mne/channels/data/montages/mgh70.elc
          ./mne/channels/data/montages/standard_1005.elc
          ./mne/channels/data/montages/standard_prefixed.elc
          ./mne/channels/data/montages/standard_alphabetic.elc
          ./mne/channels/data/montages/standard_primed.elc
          ./mne/channels/data/montages/standard_1020.elc
          ./mne/channels/data/montages/standard_postfixed.elc
          ./build/lib/mne/channels/data/montages/mgh60.elc
          ./build/lib/mne/channels/data/montages/mgh70.elc
          ./build/lib/mne/channels/data/montages/standard_1005.elc
          ./build/lib/mne/channels/data/montages/standard_prefixed.elc
          ./build/lib/mne/channels/data/montages/standard_alphabetic.elc
          ./build/lib/mne/channels/data/montages/standard_primed.elc
          ./build/lib/mne/channels/data/montages/standard_1020.elc
          ./build/lib/mne/channels/data/montages/standard_postfixed.elc
          ./sandbox/data/confidential/digitation/R0095_NMG_hsp.txt
          ./sandbox/data/confidential/digitation/R0095_NMG_elp.txt
          ./mne/channels/data/montages/biosemi160.txt
          ./mne/channels/data/montages/biosemi16.txt
          ./mne/channels/data/montages/biosemi128.txt
          ./mne/channels/data/montages/easycap-M10.txt
          ./mne/channels/data/montages/biosemi64.txt
          ./mne/channels/data/montages/easycap-M1.txt
          ./mne/channels/data/montages/biosemi256.txt
          ./mne/channels/data/montages/biosemi32.txt
          ./mne/io/tests/data/test_chpi_raw_hp.txt
          ./mne/io/tests/data/test_wrong_bads.txt
          ./mne/io/tests/data/test_bads.txt
          ./mne/io/tests/data/sample-audvis-raw-trans.txt
          ./mne/io/edf/tests/data/test_edf_stim_channel.txt
          ./mne/io/brainvision/tests/data/test_elp.txt
          ./mne/io/kit/tests/data/sns.txt
          ./mne/io/kit/tests/data/test_hsp.txt
          ./mne/io/kit/tests/data/test-eve.txt
          ./mne/io/kit/tests/data/test_elp.txt
          ./mne/io/egi/tests/data/test_egi.txt
          ./mne/datasets/_fsaverage/root.txt
          ./mne/datasets/_fsaverage/bem.txt
          ./mne/data/FreeSurferColorLUT.txt
          ./build/lib/mne/channels/data/montages/biosemi160.txt
          ./build/lib/mne/channels/data/montages/biosemi16.txt
          ./build/lib/mne/channels/data/montages/biosemi128.txt
          ./build/lib/mne/channels/data/montages/easycap-M10.txt
          ./build/lib/mne/channels/data/montages/biosemi64.txt
          ./build/lib/mne/channels/data/montages/easycap-M1.txt
          ./build/lib/mne/channels/data/montages/biosemi256.txt
          ./build/lib/mne/channels/data/montages/biosemi32.txt
          ./build/lib/mne/datasets/_fsaverage/root.txt
          ./build/lib/mne/datasets/_fsaverage/bem.txt
          ./build/lib/mne/data/FreeSurferColorLUT.txt
          ./mne/channels/data/montages/EGI_256.csd
          ./build/lib/mne/channels/data/montages/EGI_256.csd
          ./sandbox/data/confidential/digitation/R0095_NMG.elp
          ./mne/io/kit/tests/data/test.elp
          ./mne/io/edf/tests/data/biosemi.hpts
          ./mne/io/brainvision/tests/data/test.hpts
       
        mne data
          ./MNE-testing-data/EEGLAB/test_chans.locs

depends on #6765

codecov · 2019-09-13T10:07:15Z

Codecov Report

Merging #6764 into master will decrease coverage by 0.04%.
The diff coverage is 90.7%.

@@            Coverage Diff            @@
##           master   #6764      +/-   ##
=========================================
- Coverage   89.64%   89.6%   -0.05%     
=========================================
  Files         422     422              
  Lines       76545   76693     +148     
  Branches    12509   12549      +40     
=========================================
+ Hits        68621   68721     +100     
- Misses       5121    5150      +29     
- Partials     2803    2822      +19

mne/channels/tests/test_montage.py

agramfort · 2019-09-18T19:11:44Z

mne/channels/montage.py

+    Parameters
+    ----------
+    fname : str
+        The filepath of Digitization EEGLAB formatted file.


Suggested change

The filepath of Digitization EEGLAB formatted file.

The file containing electrode locations in EEGLAB format.

mne/channels/montage.py

mne/channels/tests/test_montage.py

massich · 2019-09-19T16:22:28Z

If I didn't mess up, I think that what we are missing is: hpts, loc. To me hpts seems like some other file format that has been complemented with the ch_name and kind. see https://github.com/mne-tools/mne-python/blob/master/mne/io/brainvision/tests/data/test.hpts#L3 this FastSCAN header says that the data is # x y z. Therefore the impression that ch_name and kind had been added latter. (On a side note of this file I would like to point that has a GND and RefRef channel that would suggest they are references of some sort. cross-ref #6788)

The other hpts file https://github.com/mne-tools/mne-python/blob/master/mne/io/edf/tests/data/biosemi.hpts is a biosemi64 but I don't have the format of the original file. I have not checked that the result matches make_standard_montage('biosemi64') either. This is a test that needs to be added.

the loc file we have is in datasets.testing its a theta-phi file that supposedly contains a slection of 'standard_1020'. At the beginning I was thinking about creating read_dig_eeglab but it's not really a dig but rather a "template" in the sphere. So I find it more suited to do something like read_eeglab_loc (or read_spherical_locations to be more generic and maybe we can squeeze in some of the private readers we use in make_standard_montage).

I said supposedly because the ch_names are not a subset of the 'standard_1020' we have in our templates. Our template does not contain EOGs and this selection has. Another thing that had suprised me is that the montage has 32 channels including the EOGs. I would had expect 32+2.

So my plan so far:

1 - allow polhemus to take the hpts file we have
2 - alias polhemus to also read biosemi.hpts
3 - create a function that allow to recover Theta-phi from .loc

larsoner · 2019-09-19T17:15:28Z

I would make read_dig_eeglab return the same thing that all the other read_dig_* readers do by default, namely a DigMontage that can "just be used". The fact that they store their locations a weird way is an implementation detail in my mind.

And if we want, we can add an option to return the raw theta/phi values as an ndarray if people want it.

massich · 2019-09-19T17:38:32Z

@larsoner the issue is not that it is stored weirdly. The problem is that it returns something closer to make_standard_montage rather than a polhemus subject head digitization.

agramfort · 2019-09-22T15:04:07Z

mne/channels/montage.py

+
+
+def read_standard_montage(fname, head_size=HEAD_SIZE_DEFAULT, unit='m'):
+    """Read a standard montage file containing polar coordinates.


can you check if all files you can read now are in polar coordinates?

The right way to split them would had been well possed formats vs ill-posed formats. In other wourds, if we have xyz or r,theta,phi is well posed + ch_names and types. (and we should allow for head_size=None and recover whatever is in the file with 0 magic). And this should be some kind of reader (brainvision, hpts, ..)

and read_standard_montage should be used for illposed ie not having radius.
But since we were ignoring it everywhere in read_montage. I just decided to stuff everything in read_standard_montage and drive the action with head_size.

mne/channels/montage.py

agramfort · 2019-09-22T15:09:28Z

mne/channels/tests/test_montage.py

-        montage = read_montage(op.join(tempdir, kind), transform=True)
+
+        with pytest.deprecated_call():
+            # XXX: This is a regression test that might need to be translated.


still to do or not?

massich · 2019-09-22T15:52:48Z

I'll be AFK, feel free to push in the PR.

agramfort · 2019-09-22T19:11:37Z

We need to update also https://mne.tools/dev/auto_tutorials/misc/plot_sensor_locations.html?highlight=montage

mne/channels/montage.py

* WIP: deprecate read_montage in test_montage * wip: just to see what crashes. * continue deprecation * WIP: Add read_dig_eeglab * FIX: read_dig_eeglab docstrings and doc * TST: update test * update whatsnew * add read_dig_eeglab to deprecation msg * Its not read_dig_eeglab. * WIP: Add read_dig_polhemus_fastscan * WIP: add read_standard_montage * WIP: circular dep * fix merge * wip * WIP: something is really funky * fix: old montage was not scaled for .loc * TST: I give up trying to understand why they don't get set same * fix? * remove read_dig_eeglab * fix brainvision * some more fixes * TST: make sure we can read everything * fix * FIX: make _pop_montage not depend in n_fid * WIP: add sfp * wip: add matlab * fix eeglab * wip: add asa electrode * wip: add generic theta-phi in degrees files * wip: add hpts * wip: BESA * wip: add brainvision * fix: besa * TST: add some meaningful information * TST: do test something * wip * ups * clarify what's new + fully deprecate read_dig_montage + factorize code * add read_dig_hpts fucntion * coord_frame param in DigMontage was never released * misc * pep8 + cleanup * more cleanup * update documentation * fix doc? * fix fialing test * BUG: Fix linking problems and rename function * DOC: Complete table * FIX: Dep

massich force-pushed the deprecate_montage_take2 branch from d0c3c8e to d43a8a8 Compare September 13, 2019 10:07

massich mentioned this pull request Sep 13, 2019

MRG+1, ENH: Allow creating DigMontage for standard montages #6714

Merged

massich force-pushed the deprecate_montage_take2 branch from d43a8a8 to dec9e12 Compare September 13, 2019 13:52

agramfort reviewed Sep 15, 2019

View reviewed changes

mne/channels/tests/test_montage.py Outdated Show resolved Hide resolved

larsoner added this to the 0.19 milestone Sep 15, 2019

massich force-pushed the deprecate_montage_take2 branch 2 times, most recently from 69283b0 to 7f29c02 Compare September 17, 2019 13:58

massich marked this pull request as ready for review September 17, 2019 13:58

agramfort mentioned this pull request Sep 18, 2019

Better handling of montage files #6782

Closed

massich force-pushed the deprecate_montage_take2 branch from 7f29c02 to 67cbb5d Compare September 18, 2019 15:31

agramfort reviewed Sep 18, 2019

View reviewed changes

mne/channels/montage.py Outdated Show resolved Hide resolved

agramfort reviewed Sep 18, 2019

View reviewed changes

mne/channels/tests/test_montage.py Outdated Show resolved Hide resolved

agramfort reviewed Sep 18, 2019

View reviewed changes

mne/channels/tests/test_montage.py Show resolved Hide resolved

Joan Massich and others added 9 commits September 19, 2019 16:13

WIP: deprecate read_montage in test_montage

9fedc95

wip: just to see what crashes.

7d703a7

continue deprecation

3ef0e68

WIP: Add read_dig_eeglab

a4bf378

FIX: read_dig_eeglab docstrings and doc

0b73d88

TST: update test

54d72eb

update whatsnew

71d7156

add read_dig_eeglab to deprecation msg

5d1072e

Its not read_dig_eeglab.

2c791a2

massich mentioned this pull request Sep 19, 2019

FIX: set_montage behavior for DigMontage + deprecate the rest. #6765

Merged

WIP: Add read_dig_polhemus_fastscan

e3212fd

massich force-pushed the deprecate_montage_take2 branch from e233ac5 to e3212fd Compare September 19, 2019 17:36

agramfort reviewed Sep 22, 2019

View reviewed changes

mne/channels/montage.py Outdated Show resolved Hide resolved

TST: add some meaningful information

3bea169

agramfort reviewed Sep 22, 2019

View reviewed changes

Joan Massich added 2 commits September 22, 2019 17:30

TST: do test something

ef0eac7

wip

16ab7dc

massich force-pushed the deprecate_montage_take2 branch from 7aab522 to 16ab7dc Compare September 22, 2019 15:42

ups

0bc74a0

agramfort added 6 commits September 23, 2019 13:51

clarify what's new + fully deprecate read_dig_montage + factorize code

10fc340

add read_dig_hpts fucntion

d2055ea

coord_frame param in DigMontage was never released

446c4bd

misc [ci skip]

714b4a9

pep8 + cleanup

109fc17

more cleanup

1c1cd5e

massich commented Sep 23, 2019

View reviewed changes

mne/channels/montage.py Show resolved Hide resolved

update documentation

ebddf92

agramfort mentioned this pull request Sep 23, 2019

MRG, DOC: Restore I/O content via new "tutorials" #6809

Merged

fix doc?

2823232

larsoner reviewed Sep 23, 2019

View reviewed changes

mne/channels/montage.py Outdated Show resolved Hide resolved

agramfort and others added 2 commits September 23, 2019 18:01

fix fialing test

09a619b

BUG: Fix linking problems and rename function

65bc8b9

larsoner force-pushed the deprecate_montage_take2 branch from ff5e76c to 49d7f5b Compare September 23, 2019 16:15

DOC: Complete table

d84aacf

larsoner force-pushed the deprecate_montage_take2 branch from 49d7f5b to d84aacf Compare September 23, 2019 16:22

FIX: Dep [ci skip]

2aebec4

larsoner merged commit 52f0644 into mne-tools:master Sep 23, 2019

massich deleted the deprecate_montage_take2 branch September 24, 2019 08:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate read_montage and class Montage #6764

Deprecate read_montage and class Montage #6764

massich commented Sep 13, 2019 •

edited

Loading

codecov bot commented Sep 13, 2019 •

edited

Loading

agramfort Sep 18, 2019

massich commented Sep 19, 2019 •

edited

Loading

larsoner commented Sep 19, 2019

massich commented Sep 19, 2019

agramfort Sep 22, 2019

massich Sep 22, 2019

agramfort Sep 22, 2019

massich commented Sep 22, 2019

agramfort commented Sep 22, 2019

	The filepath of Digitization EEGLAB formatted file.
	The file containing electrode locations in EEGLAB format.



		def read_standard_montage(fname, head_size=HEAD_SIZE_DEFAULT, unit='m'):
		"""Read a standard montage file containing polar coordinates.

Deprecate read_montage and class Montage #6764

Deprecate read_montage and class Montage #6764

Conversation

massich commented Sep 13, 2019 • edited Loading

codecov bot commented Sep 13, 2019 • edited Loading

Codecov Report

agramfort Sep 18, 2019

Choose a reason for hiding this comment

massich commented Sep 19, 2019 • edited Loading

larsoner commented Sep 19, 2019

massich commented Sep 19, 2019

agramfort Sep 22, 2019

Choose a reason for hiding this comment

massich Sep 22, 2019

Choose a reason for hiding this comment

agramfort Sep 22, 2019

Choose a reason for hiding this comment

massich commented Sep 22, 2019

agramfort commented Sep 22, 2019

massich commented Sep 13, 2019 •

edited

Loading

codecov bot commented Sep 13, 2019 •

edited

Loading

massich commented Sep 19, 2019 •

edited

Loading