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.

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

MRG+1: Time Frequency CSP Example #4115

Merged
merged 12 commits into from Mar 29, 2017

Conversation

Projects
None yet
4 participants
@LauraGwilliams
Contributor

LauraGwilliams commented Mar 29, 2017

Example of time-frequency decoding using CSP.

Data is band-passed at different frequencies, and the covariance across channels is computed with a sliding window.

cc @kingjr

@larsoner

Looks like a good start. Some bits of the cleaning will be necessary for CircleCI to automatically build the docs and your example. Looking forward to seeing it!

Show outdated Hide outdated examples/decoding/plot_decoding_csp_timefreq.py
# Plot results
# Set up time frequency object
av_tfr = AverageTFR(create_info(['freq'], sfreq), scores[None, :],

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

prefer explicit / future-compatible np.newaxis to None

@larsoner

larsoner Mar 29, 2017

Member

prefer explicit / future-compatible np.newaxis to None

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
from sklearn.cross_validation import ShuffleSplit # noqa
from sklearn.pipeline import Pipeline # noqa
from sklearn.cross_validation import cross_val_score # noqa
from sklearn.model_selection import ShuffleSplit

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

sklearn imports must be nested (it's an optional dependency)

@larsoner

larsoner Mar 29, 2017

Member

sklearn imports must be nested (it's an optional dependency)

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
from mne.io import concatenate_raws, read_raw_edf
from mne.datasets import eegbci
from mne.decoding import CSP
from mne.filter import band_pass_filter

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

all within-module imports should be relative instead of absolute (e.g., from ..filter import band_pass_filter

Also band_pass_filter is deprecated, you should use filter_data

@larsoner

larsoner Mar 29, 2017

Member

all within-module imports should be relative instead of absolute (e.g., from ..filter import band_pass_filter

Also band_pass_filter is deprecated, you should use filter_data

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
import numpy as np
from mne import Epochs, pick_types, find_events
from mne.io import concatenate_raws, read_raw_edf

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

you shouldn't need read_raw_edf here

@larsoner

larsoner Mar 29, 2017

Member

you shouldn't need read_raw_edf here

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
@@ -0,0 +1,186 @@
# laura gwilliams leg5@nyu.edu
# run classifier at different time and frequency windows

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

please add our standard boilerplate stuff at the top

@larsoner

larsoner Mar 29, 2017

Member

please add our standard boilerplate stuff at the top

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
from sklearn.model_selection import ShuffleSplit
class TimeFrequencyCSP():

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

needs standard docstring (NumPy style -- see other classes for examples)

@larsoner

larsoner Mar 29, 2017

Member

needs standard docstring (NumPy style -- see other classes for examples)

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
self.tmax = tmax
self._csp = [] # make this a numpy array not a list
def transform(self, X, decim=None):

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

same thing -- public methods must have docstring

@larsoner

larsoner Mar 29, 2017

Member

same thing -- public methods must have docstring

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
# preprocessing here
def time_frequency_preprocess(raw_fnames, freq_bins, n_cycles, tmin, tmax,

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

let's make this private by prefixing the name with _

@larsoner

larsoner Mar 29, 2017

Member

let's make this private by prefixing the name with _

Show outdated Hide outdated mne/decoding/_time_frequency_csp.py
return epoch_list, epochs.events[:, 2]
tmin, tmax = -0.5, 4.

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

none of this stuff should be here (it's in the example effectively, right?)

@larsoner

larsoner Mar 29, 2017

Member

none of this stuff should be here (it's in the example effectively, right?)

Show outdated Hide outdated examples/decoding/plot_decoding_csp_timefreq.py
from mne.decoding import CSP
from mne.time_frequency import AverageTFR
from sklearn.lda import LDA

This comment has been minimized.

@agramfort

agramfort Mar 29, 2017

Member

use new version

from sklearn.discriminant_analysis import LinearDiscriminantAnalysis

@agramfort

agramfort Mar 29, 2017

Member

use new version

from sklearn.discriminant_analysis import LinearDiscriminantAnalysis

Show outdated Hide outdated examples/decoding/plot_decoding_csp_timefreq.py
centered_w_times, freqs[1:], 1)
chance = np.mean(y) # set chance level to white in the plot
av_tfr.plot([0], vmin=chance, title="Time-Frequency Decoding Scores")

This comment has been minimized.

@agramfort

agramfort Mar 29, 2017

Member

av_tfr.plot([0], vmin=chance, title="Time-Frequency Decoding Scores", cmap=plt.cm.Reds)

@agramfort

agramfort Mar 29, 2017

Member

av_tfr.plot([0], vmin=chance, title="Time-Frequency Decoding Scores", cmap=plt.cm.Reds)

Show outdated Hide outdated examples/decoding/plot_decoding_csp_timefreq.py
# Classification & Time-frequency parameters
tmin, tmax = -.200, 2.000
n_cycles = 15. # how many complete cycles: used to define window size
min_freq = 5.

This comment has been minimized.

@agramfort

agramfort Mar 29, 2017

Member

15 cycles are 5Hz seems huge

@agramfort

agramfort Mar 29, 2017

Member

15 cycles are 5Hz seems huge

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Mar 29, 2017

Member

besides LGTM

Member

agramfort commented Mar 29, 2017

besides LGTM

# Alex Barachant <alexandre.barachant@gmail.com>
# Alexandre Gramfort <alexandre.gramfort@telecom-paristech.fr>
#
# License: BSD (3-clause)

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

This example needs a docstring (see the related CircleCI failure), check out any of the other examples for the format

@larsoner

larsoner Mar 29, 2017

Member

This example needs a docstring (see the related CircleCI failure), check out any of the other examples for the format

Show outdated Hide outdated examples/decoding/plot_decoding_csp_timefreq.py
from sklearn.pipeline import make_pipeline
# #############################################################################

This comment has been minimized.

@larsoner

larsoner Mar 29, 2017

Member

It doesn't quite render the sections correctly, I think because these breaks are not formatted properly

https://4168-1301584-gh.circle-artifacts.com/0/home/ubuntu/mne-python/doc/_build/html/auto_examples/decoding/plot_decoding_csp_timefreq.html

I think there should be no empty newline here, and that the # ##### above should be ###### (no space), but check existing examples to make sure

@larsoner

larsoner Mar 29, 2017

Member

It doesn't quite render the sections correctly, I think because these breaks are not formatted properly

https://4168-1301584-gh.circle-artifacts.com/0/home/ubuntu/mne-python/doc/_build/html/auto_examples/decoding/plot_decoding_csp_timefreq.html

I think there should be no empty newline here, and that the # ##### above should be ###### (no space), but check existing examples to make sure

LauraGwilliams added some commits Mar 29, 2017

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Mar 29, 2017

Member

@LauraGwilliams can you update what's new?

Member

agramfort commented Mar 29, 2017

@LauraGwilliams can you update what's new?

@LauraGwilliams

This comment has been minimized.

Show comment
Hide comment
@LauraGwilliams

LauraGwilliams Mar 29, 2017

Contributor

@agramfort yes, will do now!

Contributor

LauraGwilliams commented Mar 29, 2017

@agramfort yes, will do now!

@larsoner larsoner changed the title from ENH: Time Frequency CSP Example to MRG+1: Time Frequency CSP Example Mar 29, 2017

@agramfort agramfort merged commit f823a00 into mne-tools:master Mar 29, 2017

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci Your tests passed on CircleCI!
Details
@agramfort

This comment has been minimized.

Show comment
Hide comment
Member

agramfort commented Mar 29, 2017

@LauraGwilliams

This comment has been minimized.

Show comment
Hide comment
@LauraGwilliams

LauraGwilliams Mar 29, 2017

Contributor

Great, thank you for your comments!

Contributor

LauraGwilliams commented Mar 29, 2017

Great, thank you for your comments!

@kingjr kingjr referenced this pull request Mar 31, 2017

Open

ENH: decoding module 2017 #3442

26 of 38 tasks complete
@larsoner

This comment has been minimized.

Show comment
Hide comment
@larsoner

larsoner Apr 2, 2017

Member

@LauraGwilliams we need a URL for you in whats_new.rst, have a preferred one?

Member

larsoner commented Apr 2, 2017

@LauraGwilliams we need a URL for you in whats_new.rst, have a preferred one?

@LauraGwilliams

This comment has been minimized.

Show comment
Hide comment
@LauraGwilliams

LauraGwilliams Apr 3, 2017

Contributor

@Eric89GXL yes, this one probably http://lauragwilliams.github.io. Thanks!

Contributor

LauraGwilliams commented Apr 3, 2017

@Eric89GXL yes, this one probably http://lauragwilliams.github.io. Thanks!

@larsoner larsoner added mne sprint and removed mne sprint labels Apr 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment