Skip to content

MRG plot_topomap from info #3165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jaeilepp merged 9 commits into from
Apr 25, 2016
Merged

MRG plot_topomap from info #3165

jaeilepp merged 9 commits into from
Apr 25, 2016

Conversation

@jona-sassenhagen
Copy link
Contributor

@jona-sassenhagen jona-sassenhagen commented Apr 23, 2016 •
edited
Loading

Closes #3078

Try:

import mne
data_path = mne.datasets.sample.data_path()
evoked_fname = data_path + '/MEG/sample/sample_audvis-ave.fif'
for type in ('mag', 'grad'):
    evoked = mne.read_evokeds(evoked_fname, condition=1).pick_types(eeg=False, meg=type).detrend()
    arr = evoked.data[:, 171]
    mne.viz.plot_topomap(arr, evoked.info);

@jona-sassenhagen jona-sassenhagen changed the title plot_topomap from info WIP plot_topomap from info Apr 23, 2016
@coveralls
Copy link

coveralls commented Apr 23, 2016

Coverage Status

Coverage increased (+0.002%) to 90.822% when pulling cb21ff2 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

agramfort
agramfort reviewed Apr 23, 2016
View reviewed changes
mne/viz/topomap.py Outdated
else:
ch_type = ch_type.pop()

if any(type in ch_type for type in ('planar', 'grad')):
Copy link
Member

@agramfort agramfort Apr 23, 2016

Choose a reason for hiding this comment

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

Don't overwrite built in type

@agramfort
Copy link
Member

agramfort commented Apr 23, 2016

Looks pretty clean

See why Travis complains

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 23, 2016 •
edited
Loading

Travis is fine, Appveyor is:

Build started
git clone -q https://github.com/mne-tools/mne-python.git C:\projects\mne-python
fatal: unable to access 'https://github.com/mne-tools/mne-python.git/': Failed to connect to github.com port 443: Timed out
Command exited with code 128

I think that's unrelated?

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 23, 2016

@wmvanvliet @kingjr what do you think of the API?

kingjr
kingjr reviewed Apr 23, 2016
View reviewed changes
mne/viz/topomap.py Outdated
raise ValueError("Multiple channel types in Info structure. " +
info_help)
elif len(pos["chs"]) != data.shape[0]:
raise ValueError("Number of channels in Info and Data does not "
Copy link
Member

@kingjr kingjr Apr 23, 2016

Choose a reason for hiding this comment

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

Incorrect var naming ?

@kingjr
Copy link
Member

kingjr commented Apr 23, 2016

I just have my phone but it seems OK to me

@coveralls
Copy link

coveralls commented Apr 23, 2016

Coverage Status

Coverage increased (+0.0005%) to 90.82% when pulling 67b42c0 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

@jona-sassenhagen jona-sassenhagen changed the title WIP plot_topomap from info MRG plot_topomap from info Apr 24, 2016
@coveralls
Copy link

coveralls commented Apr 24, 2016

Coverage Status

Coverage decreased (-0.003%) to 90.816% when pulling 2823ae2 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

larsoner
larsoner reviewed Apr 24, 2016
View reviewed changes
mne/viz/tests/test_topomap.py
# Plot array
for ch_type in ('mag', 'grad'):
evoked_ = evoked.copy().pick_types(eeg=False, meg=ch_type)
plot_topomap(evoked_.data[:, 0], evoked_.info)
Copy link
Member

@larsoner larsoner Apr 24, 2016

Choose a reason for hiding this comment

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

Does it also work with evoked_.data[:, [0]] which has shape (n_channels, 1) and evoked_.data[:, :3] which has multiple time points? It's not clear from the tests if those should work or not.

Copy link
Contributor Author

@jona-sassenhagen jona-sassenhagen Apr 24, 2016

Choose a reason for hiding this comment

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

Well I don't know but this PR doesn't change any of this. The PR doesn't touch anything beyond getting the pos array.

@larsoner larsoner added this to the 0.12 milestone Apr 24, 2016
@coveralls
Copy link

coveralls commented Apr 24, 2016

Coverage Status

Coverage decreased (-0.001%) to 90.818% when pulling 2823ae2 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

@coveralls
Copy link

coveralls commented Apr 24, 2016

Coverage Status

Coverage decreased (-0.001%) to 90.818% when pulling 6f59c76 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

@coveralls
Copy link

coveralls commented Apr 24, 2016

Coverage Status

Coverage decreased (-0.003%) to 90.816% when pulling fbe19db on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 24, 2016

appveyor failure seems unrelated. @Eric89GXL can you restart?

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 24, 2016

Good from my end.

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 24, 2016

Maybe wait with merge for comments by @wmvanvliet and/or @kingjr ?..

@wmvanvliet
Copy link
Contributor

wmvanvliet commented Apr 25, 2016

Looks good to me!

@jaeilepp
Copy link
Contributor

jaeilepp commented Apr 25, 2016

LGTM

@agramfort
Copy link
Member

agramfort commented Apr 25, 2016

update what's new and merge.

thx @jona-sassenhagen

@coveralls
Copy link

coveralls commented Apr 25, 2016

Coverage Status

Coverage decreased (-0.007%) to 90.813% when pulling 44aff47 on jona-sassenhagen:topomap_array into 20de22d on mne-tools:master.

@jona-sassenhagen
Copy link
Contributor Author

jona-sassenhagen commented Apr 25, 2016

Hit the button! Thanks for reviews everyone.

@jaeilepp jaeilepp merged commit 4badef6 into mne-tools:master Apr 25, 2016
@jaeilepp
Copy link
Contributor

jaeilepp commented Apr 25, 2016

Thanks @jona-sassenhagen

@jona-sassenhagen jona-sassenhagen deleted the topomap_array branch April 25, 2016 13:19
@jona-sassenhagen jona-sassenhagen mentioned this pull request Jun 3, 2016
Merged
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.12

Development

Successfully merging this pull request may close these issues.

7 participants

@jona-sassenhagen @coveralls @agramfort @kingjr @wmvanvliet @jaeilepp @larsoner