Skip to content

Conversation

@hoechenberger
Copy link
Member

Fixes #8334

@hoechenberger hoechenberger force-pushed the hoechenberger/issue8334 branch from f4437a6 to 9ed2040 Compare March 29, 2021 17:01
@hoechenberger hoechenberger marked this pull request as ready for review March 29, 2021 17:02
@hoechenberger hoechenberger changed the title WIP: Add Evoked.baseline ENH: Add Evoked.baseline Mar 29, 2021
@hoechenberger
Copy link
Member Author

This is ready for review. The PR adds an Evoked.baseline attribute, which is derived from Epochs.baseline during averaging.

@hoechenberger
Copy link
Member Author

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

baseline = (-0.2, -0.1)
evoked.apply_baseline(baseline)
with pytest.raises(ValueError, match='already been baseline-corrected'):
evoked.apply_baseline(None)
Copy link
Member

Choose a reason for hiding this comment

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

I don't see a assert evoked.baseline is None in any of these tests, can you add a test that no baseline gets this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I've added a few more test cases in 21d5514

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

LGTM +1 for merge

@hoechenberger hoechenberger merged commit 53a8634 into mne-tools:main Mar 30, 2021
@hoechenberger hoechenberger deleted the hoechenberger/issue8334 branch March 30, 2021 06:44
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.

Add Evoked.baseline

2 participants