Skip to content

MNT, TST: clarify existing behavior of set_montage / __init__ montage#6529

Merged
larsoner merged 10 commits intomne-tools:masterfrom
massich:eeglab_set_montage
Jul 5, 2019
Merged

MNT, TST: clarify existing behavior of set_montage / __init__ montage#6529
larsoner merged 10 commits intomne-tools:masterfrom
massich:eeglab_set_montage

Conversation

@massich
Copy link
Copy Markdown
Contributor

@massich massich commented Jul 4, 2019

So far this PR rewrites an existing test to discuss which is the desired behaviour

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 4, 2019

Codecov Report

Merging #6529 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #6529      +/-   ##
==========================================
- Coverage   89.31%   89.26%   -0.05%     
==========================================
  Files         413      413              
  Lines       74532    74556      +24     
  Branches    12293    12297       +4     
==========================================
- Hits        66565    66552      -13     
- Misses       5126     5161      +35     
- Partials     2841     2843       +2

Joan Massich added 2 commits July 4, 2019 19:46
Copy link
Copy Markdown
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

good to go?

_assert_array_equal_nan(np.array([ch['loc'] for ch in foo.info['chs']]),
EXPECTED_LOCATIONS_FROM_MONTAGE)

# flushing
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do you say flushing here?

def _assert_array_equal_nan(left, right):
# from mne.utils import _array_equal_nan
# assert _array_equal_nan(np.array([ch['loc'] for ch in raw.info['chs']]),
# EXPECTED_LOCATIONS_FROM_MONTAGE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cleanup?

@massich
Copy link
Copy Markdown
Contributor Author

massich commented Jul 5, 2019

I guess it can go in. But my main goal with this PR was to unfold the behavior of the set_montage.

@massich massich changed the title FIX: set_montage / __init__ montage behavior when missing positions MNT, TST: clarify existing behavior of set_montage / __init__ montage Jul 5, 2019
@massich massich marked this pull request as ready for review July 5, 2019 09:35
massich pushed a commit to massich/mne-python that referenced this pull request Jul 5, 2019
@larsoner larsoner merged commit 5bc8457 into mne-tools:master Jul 5, 2019
massich pushed a commit that referenced this pull request Jul 5, 2019
massich pushed a commit to massich/mne-python that referenced this pull request Jul 5, 2019
@massich massich deleted the eeglab_set_montage branch August 7, 2019 10:05
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