Skip to content
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

WIP: preprocessing module #40

Merged
merged 11 commits into from
May 4, 2012
Merged

Conversation

mluessi
Copy link
Contributor

@mluessi mluessi commented Apr 26, 2012

@agramfort: first rework for the preprocessing module. Let me know if you think things should be done differently.

@agramfort
Copy link
Member

Some files are missing I think

@mluessi
Copy link
Contributor Author

mluessi commented Apr 26, 2012

doh.. silly me, the files are here now

@@ -116,10 +38,10 @@ def compute_proj_ecg(in_fif_fname, tmin, tmax, n_grad, n_mag, n_eeg, l_freq,
default=2)
parser.add_option("--l-freq", dest="l_freq",
help="Filter low cut-off frequency in Hz",
default=None) # XXX
default=5)
Copy link
Member

Choose a reason for hiding this comment

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

isn't it a bit high?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, changed it to 1 Hz

@agramfort
Copy link
Member

for maxfiler I would go for mne_maxfilter.py with resonable default parameters

@mluessi
Copy link
Contributor Author

mluessi commented Apr 30, 2012

@agramfort have a look, I added mne_maxfilter and EOG

mne.preprocessing.compute_proj_ecg(raw_in, tmin, tmax,
n_grad, n_mag, n_eeg, l_freq, h_freq, average, preload,
filter_length, n_jobs, ch_name, reject, bads,
avg_ref, include_existing, ecg_proj_fname, ecg_event_fname)
Copy link
Member

Choose a reason for hiding this comment

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

I think raw_in should be an instance of mne.fiff.Raw and the function should return a list of Projection

the save would be done here with mne.write_proj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I didn't use mne.fiff.Raw as input is that the function modifies the data, if someone uses compute_proj_ecg directly from python this could be a problem.

I can save the events and proj here, no problem.

Copy link
Member

Choose a reason for hiding this comment

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

The reason why I didn't use  mne.fiff.Raw as input is that the function modifies the data, if someone uses  compute_proj_ecg directly from python this could be a problem.

ok but you may want to do something on the raw before using
compute_proj_ecg like filtering, marking bad channels etc. I don't
think modifying the data is a problem if it's clear in the doc string
and we could have a copy parameter as in numpy

I can save the events and proj here, no problem.

good

@agramfort
Copy link
Member

we should have a test_ssp.py that tests the compute_proj_ecg and compute_proj_eog functions

the artifacts module should be integrated into the preprocessing module

@agramfort
Copy link
Member

tests are important to keep a good code coverage :)

that's it for tonight ;)

@mluessi
Copy link
Contributor Author

mluessi commented May 3, 2012

@agramfort: have a look, I think this covers everything.

@agramfort
Copy link
Member

looks great ! thanks

I'll try the script on a couple of datasets tomorrow and I'll merge

Martin Luessi and others added 3 commits May 3, 2012 16:49
mluessi pushed a commit that referenced this pull request May 4, 2012
@mluessi mluessi merged commit 676354b into mne-tools:master May 4, 2012
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.

2 participants