FIX: set_montage behavior for DigMontage + deprecate the rest. by massich · Pull Request #6765 · mne-tools/mne-python

massich · 2019-09-13T09:01:12Z

This PR reviews the behavior of setting a DigMontage:

1 - what happens if info['ch_names'] and montage.ch_names are different versions of the same (i.e: 'EEG 001' and 'EEG001')?
crashes. This needs to be taken care of by the user.

2 - what happens when montage is a sub/super set of info?
* supper set case: warns, sets up info['ch_pos'] and info['dig'] for the ch_names already in info
* subset case: crashes.

3 - requires montage to be in 'head' (or being able to be transformed in 'head'). Crashes otherwise.

cc: @agramfort, @larsoner

codecov · 2019-09-13T12:18:35Z

Codecov Report

Merging #6765 into master will increase coverage by 1.3%.
The diff coverage is 97.95%.

@@            Coverage Diff            @@
##           master    #6765     +/-   ##
=========================================
+ Coverage   88.23%   89.53%   +1.3%     
=========================================
  Files         421      422      +1     
  Lines       76301    76408    +107     
  Branches    12478    12490     +12     
=========================================
+ Hits        67325    68415   +1090     
+ Misses       6174     5183    -991     
- Partials     2802     2810      +8

mne/channels/montage.py

mne/channels/tests/test_montage.py

mne/io/tests/test_meas_info.py

lgtm-com · 2019-09-16T14:03:58Z

This pull request introduces 1 alert when merging a3c7d69 into 2545faf - view on LGTM.com

new alerts:

1 for Wrong number of arguments in a call

mne/channels/tests/test_montage.py

examples/datasets/plot_oldfiles_set_montage.py

mne/channels/montage.py

mne/channels/tests/test_montage.py

mne/io/tests/test_meas_info.py

examples/datasets/plot_oldfiles_set_montage.py

massich · 2019-09-19T15:38:08Z

@larsoner This should be good to go, can you do a pass? Thx

This PR only deprecates parts of set_montage and fixes the bihavior of set_montage when using DigMontage.

massich · 2019-09-19T16:25:52Z

This PR blocks #6764

massich · 2019-09-19T16:29:06Z

Does anyone know how can I pinpoint set_montage of RawEEGLAB in the whats new?
cc: @drammock

larsoner · 2019-09-19T17:10:55Z

Failures appear real

https://travis-ci.org/mne-tools/mne-python/jobs/587075768

doc/changes/latest.inc

mne/channels/montage.py

mne/channels/tests/test_montage.py

larsoner · 2019-09-19T17:58:12Z

You are probably running a recent enough Python that dict iteration order is guaranteed. On 3.5 it's not

mne/io/tests/test_meas_info.py

agramfort

besides this LGTM +1 for MRG if CIs are happy

mne/io/eeglab/tests/test_eeglab.py

mne/io/eeglab/tests/_utils.py

mne/io/eeglab/tests/test_eeglab.py

…ools#6765) * TST: reveal issues * TST: Add a test describing current status * Add example modifying montage names * TST: Add test showing how to set mismatching montages * TST: set up the sub/super set behavior * WIP: set_montage for DigMontage (with and without ref) * ENH: use view * TST, BUG-FIX: Adapt tests to the new set_montage. (new is good) * WIP, TST: start working with getting a propper info_'dig'_ * wip (I'm doing something wrong) * wip * done. todo: fix deprecationwarn in generator * fix: StopIteration Deprecation warning * wip * FIX: HPI was FIFF_EEG_CH, but somewhere I've seen EEEG_REF being same * clean up * WIP: remove the generator * check set_dig * FIX: test_fif_dig_montage * WIP: bypass set_montage in egi test * WIP: bypass set_montage in bvct * wip: fix bvct (second test) * WIP: better error handling. it will simplify the TST + backcompat * wip * WIP: I can do pick_types with including the bad channels * Fix * FIX: just selecting all eeg_like should make the trick * WIP: Add transformation to head in set_montage when necessary * TST: revert the funky _egi_set_montage and do a pass on the egi test * TST: revert the funky _set_montage for bvct * WIP: fix some * FIX: fieldtrip, but it might be a BUG * wip * FIX: avoid using _coord_frame * remove example * WIP: Add deprecation * FIX some deprecations * BUG: Brainvision * wip * wip * WIP: use _fix_montage in eeglab * wip * wip * wip * wip: eeglab * wip: fixing more * revert * montage is not needed to test ica * ups I deleted the wrong file * FIX: deprecations * thats funny * update whats_new * clean-up * FIX: A few more * FIX: Old NumPy * FIX: More * FIX: Old NumPy is annoying

massich mentioned this pull request Sep 13, 2019

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

Merged

massich force-pushed the set_digmontage_when_chnames_mismatch branch from 4a4a12f to 6598d41 Compare September 13, 2019 09:40

massich force-pushed the set_digmontage_when_chnames_mismatch branch 2 times, most recently from b05bd2c to 1e11b77 Compare September 13, 2019 13:58

massich commented Sep 13, 2019

View reviewed changes

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

larsoner reviewed Sep 14, 2019

View reviewed changes

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

agramfort reviewed Sep 14, 2019

View reviewed changes

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

agramfort reviewed Sep 14, 2019

View reviewed changes

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

agramfort reviewed Sep 14, 2019

View reviewed changes

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

agramfort reviewed Sep 14, 2019

View reviewed changes

mne/io/tests/test_meas_info.py Outdated Show resolved Hide resolved

larsoner added this to the 0.19 milestone Sep 15, 2019

massich force-pushed the set_digmontage_when_chnames_mismatch branch 3 times, most recently from f2f0231 to 0469e8b Compare September 16, 2019 12:11

massich commented Sep 16, 2019

View reviewed changes

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

massich force-pushed the set_digmontage_when_chnames_mismatch branch 2 times, most recently from 7cc31bc to 2fc5963 Compare September 17, 2019 15:18

agramfort reviewed Sep 17, 2019

View reviewed changes

examples/datasets/plot_oldfiles_set_montage.py Outdated Show resolved Hide resolved

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/montage.py Outdated Show resolved Hide resolved

agramfort reviewed Sep 18, 2019

View reviewed changes

mne/channels/montage.py 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 Outdated Show resolved Hide resolved

agramfort reviewed Sep 18, 2019

View reviewed changes

mne/io/tests/test_meas_info.py Outdated Show resolved Hide resolved

massich force-pushed the set_digmontage_when_chnames_mismatch branch 2 times, most recently from 859d6f8 to 021198f Compare September 18, 2019 16:26

jasmainak reviewed Sep 18, 2019

View reviewed changes

examples/datasets/plot_oldfiles_set_montage.py Outdated Show resolved Hide resolved

massich changed the title ~~[RFC] Setup DigMontage to mismatching raw.ch_names~~ FIX: set_montage behavior for DigMontage + deprecate the rest. Sep 19, 2019

Joan Massich added 8 commits September 19, 2019 16:12

wip: eeglab

bd097f5

wip: fixing more

f7ec7a3

revert

a7897ae

montage is not needed to test ica

89c35c1

ups I deleted the wrong file

2233859

FIX: deprecations

8822407

thats funny

4de2d0d

update whats_new

60db67e

massich force-pushed the set_digmontage_when_chnames_mismatch branch from 3326cff to bbbf5f6 Compare September 19, 2019 14:13

clean-up

e7f05a2

massich force-pushed the set_digmontage_when_chnames_mismatch branch from bbbf5f6 to e7f05a2 Compare September 19, 2019 15:35

massich mentioned this pull request Sep 19, 2019

Deprecate read_montage and class Montage #6764

Merged

larsoner reviewed Sep 19, 2019

View reviewed changes

doc/changes/latest.inc Outdated Show resolved Hide resolved

mne/channels/montage.py Show resolved Hide resolved

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

larsoner mentioned this pull request Sep 19, 2019

MRG, ENH: Make use of fiducials #6791

Merged

8 tasks

larsoner reviewed Sep 19, 2019

View reviewed changes

mne/io/tests/test_meas_info.py Outdated Show resolved Hide resolved

larsoner added 2 commits September 19, 2019 14:15

FIX: A few more

45b97e8

FIX: Old NumPy

046e289

larsoner force-pushed the set_digmontage_when_chnames_mismatch branch from 23aa211 to 046e289 Compare September 19, 2019 19:05

FIX: More

5d77fb1

agramfort reviewed Sep 19, 2019

View reviewed changes

mne/io/eeglab/tests/test_eeglab.py Outdated Show resolved Hide resolved

mne/io/eeglab/tests/_utils.py Show resolved Hide resolved

mne/io/eeglab/tests/test_eeglab.py Outdated Show resolved Hide resolved

FIX: Old NumPy is annoying

d338174

larsoner merged commit f1f8e54 into mne-tools:master Sep 19, 2019

larsoner mentioned this pull request Sep 19, 2019

WIP: Fix broken examples #6793

Merged

massich deleted the set_digmontage_when_chnames_mismatch branch September 24, 2019 08:02

Uh oh!

Conversation

massich commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Sep 16, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

massich commented Sep 19, 2019

Uh oh!

massich commented Sep 19, 2019

Uh oh!

massich commented Sep 19, 2019

Uh oh!

larsoner commented Sep 19, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsoner commented Sep 19, 2019

Uh oh!

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

massich commented Sep 13, 2019 •

edited

Loading

codecov bot commented Sep 13, 2019 •

edited

Loading