Skip to content

MRG : EEG Reference reloaded ! #1713

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
Closed
agramfort wants to merge 12 commits into from
Closed

MRG : EEG Reference reloaded ! #1713

agramfort wants to merge 12 commits into from

Conversation

@agramfort
Copy link
Member

@agramfort agramfort commented Dec 23, 2014

No description provided.

wmvanvliet and others added 11 commits December 22, 2014 10:06
@wmvanvliet
Add more referencing options
372b18e 
This adds a base function 'apply_reference' that can apply various
different referencing schemes. Two convenience functions are also
added that provide a convenient interface to the base function:

 - set_eeg_reference: apply a simple reference to all EEG channels
 - set_bipolar_reference: apply a bipolar reference (or more)
@wmvanvliet
Add flag that a custom reference has been applied
82ebc6e 
Raw and Epoch classes will no longer add an average reference if the
`custom_ref_applied` flag was set. SSP projections will still attempt
this, but a new assertion in `make_eeg_average_ref_proj` will raise an
error if the flag was set.

Before a CAR projection is added, the `info['custom_ref_applied']` flag
is checked.
@wmvanvliet
Add default channel name for bipolar references
ff0b1eb 
- Also check for duplicate channel names
- Bilpolar channels can be named after the anode/cathode
- Inform user of the chosen bipolar channel name
@wmvanvliet
Updating ref and what's new
c0a00a8
@wmvanvliet
Some improvements to the docs
51ac222
@wmvanvliet
Make apply_reference a private function
21c2227
@wmvanvliet
Reduced memory consumption of rerefencing example
cad7e35
@agramfort
cosmit in example
f4f0e6f
@agramfort
move EEG referencing code to separate file
0b008a9
@agramfort
typos
8fa8dc9
@agramfort
prevent inverse modeling with bad or no EEG reference
8e150c2
@agramfort agramfort mentioned this pull request Dec 23, 2014
Closed
@agramfort agramfort changed the title EEG Reference reloaded ! MRG : EEG Reference reloaded ! Dec 23, 2014
@agramfort
Copy link
Member Author

agramfort commented Dec 23, 2014

ready to merge on my end

@wmvanvliet
Copy link
Contributor

wmvanvliet commented Dec 23, 2014

_has_avg_ref_proj and _needs_avg_ref_proj may be good candidates to move to io/reference.py. Although they deal with projections, so keeping them in proj.py makes sense as well... your call.

I'm very happy with these changes.

@agramfort
Copy link
Member Author

agramfort commented Dec 24, 2014

I can move these functions easily otherwise it creates circular dependencies between proj.py and reference.py

@wmvanvliet let me give you write access to the repo so you can click the green button yourself :)

@dengemann
Copy link
Member

dengemann commented Dec 24, 2014

I'll have some time to do one round of review and testing. I'd like to see how it works with EGI data

On 24 Dec 2014, at 11:31, Alexandre Gramfort notifications@github.com wrote:

I can move these functions easily otherwise it creates circular dependencies between proj.py and reference.py

@wmvanvliet let me give you write access to the repo so you can click the green button yourself :)

—
Reply to this email directly or view it on GitHub.

This was referenced Dec 24, 2014
Closed
Closed
@larsoner
Copy link
Member

larsoner commented Dec 24, 2014

LGTM, +1 for merge once @dengemann is happy.

@agramfort
Copy link
Member Author

agramfort commented Dec 25, 2014

let's wait for @dengemann then. No rush it's xmas...

@wmvanvliet
Copy link
Contributor

wmvanvliet commented Dec 25, 2014

Merry Christmas everyone.

(why is my inbox exploding with mne-python comments/commits? Stop coding and go celebrate ;])

On 25 Dec 2014, at 18:49, Alexandre Gramfort notifications@github.com wrote:

let's wait for @dengemann then. No rush it's xmas...
—
Reply to this email directly or view it on GitHub #1713 (comment).

Marijn van Vliet
w.m.vanvliet@gmail.com

@agramfort
Copy link
Member Author

agramfort commented Dec 25, 2014

merry MNE xmas ! :)

@dengemann
Copy link
Member

dengemann commented Dec 25, 2014

Santa brought us a good hacking time :) so many presents waiting for merge

2014-12-25 18:44 GMT+01:00 Alexandre Gramfort notifications@github.com:

merry MNE xmas ! :)

—
Reply to this email directly or view it on GitHub
#1713 (comment).

@dengemann
Copy link
Member

dengemann commented Dec 28, 2014

@wmvanvliet this one needs a rebase.

dengemann
dengemann reviewed Dec 28, 2014
View reviewed changes
mne/io/proj.py Outdated
Copy link
Member

@dengemann dengemann Dec 28, 2014

Choose a reason for hiding this comment

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

@wmvanvliet for python 3 compat it would be good to use six string_types here.

@wmvanvliet wmvanvliet mentioned this pull request Jan 5, 2015
Merged
@wmvanvliet
Copy link
Contributor

wmvanvliet commented Jan 5, 2015

closed in favour of #1733

@wmvanvliet wmvanvliet closed this Jan 5, 2015
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

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@agramfort @wmvanvliet @dengemann @larsoner