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.

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

MRG: Cleaner epochs processing and delayed tests #2441

Merged
merged 3 commits into from
Aug 31, 2015
Merged

MRG: Cleaner epochs processing and delayed tests #2441

merged 3 commits into from
Aug 31, 2015

Conversation

larsoner
Copy link
Member

Closes #2440.

@larsoner larsoner force-pushed the delayed branch 2 times, most recently from 5fa163f to e11f37a Compare August 28, 2015 20:35
@larsoner larsoner changed the title FIX: Cleaner epochs processing and delayed tests MRG: Cleaner epochs processing and delayed tests Aug 28, 2015
@larsoner
Copy link
Member Author

Ready for review/merge from my end.

@agramfort
Copy link
Member

looks clean and a good step forward the simplification of the Epochs class.

@Eric89GXL did you run the test with interactive proj to see if it still behaves fine?

@larsoner
Copy link
Member Author

You mean the example examples/visualization/plot_evoked_topomap_delayed_ssp.py? It seems to run the same on both master and this branch, yeah. Projs can be interactively toggled to see the results.

agramfort
agramfort reviewed Aug 30, 2015
View reviewed changes
mne/tests/test_epochs.py
np.array(epochs.ch_names)[picks_data])
assert_allclose(evoked.times, epochs.times)
assert_allclose(evoked.data, epochs_data,
rtol=1e-5, atol=1e-15)
Copy link
Member

Choose a reason for hiding this comment

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

this test would fail in master? how long does it take to run? is it relevant to use less events or it's already fast enough?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, fails in master. About 4 sec on my machine, but I might be able to make it faster. I'll try tomorrow.

Copy link
Member

Choose a reason for hiding this comment

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

@larsoner
Copy link
Member Author

@agramfort done

agramfort added a commit that referenced this pull request Aug 31, 2015
@agramfort
Merge pull request #2441 from Eric89GXL/delayed
57a5333 
MRG: Cleaner epochs processing and delayed tests
@agramfort agramfort merged commit 57a5333 into mne-tools:master Aug 31, 2015
@agramfort
Copy link
Member

thanks @Eric89GXL !

@larsoner larsoner deleted the delayed branch September 18, 2015 16:22
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.
2 participants
@larsoner @agramfort