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

Refactor of NoisyChannels tests and split into separate methods #84

Merged
merged 7 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 50 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,60 @@ def montage():

@pytest.fixture(scope="session")
def raw():
"""Fixture for physionet EEG subject 4, dataset 1."""
"""Return an `mne.io.Raw` object for use with unit tests.

This fixture downloads and reads in subject 4, run 1 from the Physionet
BCI2000 (eegbci) open dataset. This recording is quite noisy and is thus a
good candidate for testing the PREP pipeline.

File attributes:
- Channels: 64 EEG
- Sample rate: 160 Hz
- Duration: 61 seconds

This is only run once per session to save time downloading.

"""
mne.set_log_level("WARNING")
# load in subject 1, run 1 dataset

# Download and read S004R01.edf from the BCI2000 dataset
edf_fpath = eegbci.load_data(4, 1, update_path=True)[0]
raw = mne.io.read_raw_edf(edf_fpath, preload=True)
eegbci.standardize(raw) # Fix non-standard channel names

return raw


# using sample EEG data (https://physionet.org/content/eegmmidb/1.0.0/)
@pytest.fixture(scope="session")
def raw_clean(montage):
"""Return an `mne.io.Raw` object with no bad channels for use with tests.

This fixture downloads and reads in subject 1, run 1 from the Physionet
BCI2000 (eegbci) open dataset, interpolates its two bad channels (T10 & F8),
sappelhoff marked this conversation as resolved.
Show resolved Hide resolved
and performs average referencing on the data. Intended for use with tests
where channels are made artifically bad.

File attributes:
- Channels: 64 EEG
- Sample rate: 160 Hz
- Duration: 61 seconds

This is only run once per session to save time downloading.

"""
mne.set_log_level("WARNING")

# Download and read S001R01.edf from the BCI2000 dataset
edf_fpath = eegbci.load_data(1, 1, update_path=True)[0]
raw = mne.io.read_raw_edf(edf_fpath, preload=True)
eegbci.standardize(raw) # Fix non-standard channel names

# Interpolate the file's few bad channels to produce a clean dataset
raw.set_montage(montage)
raw.info["bads"] = ["T10", "F8"]
raw.interpolate_bads()

# The eegbci data has non-standard channel names. We need to rename them:
eegbci.standardize(raw)
# Re-reference the data after interpolating bad channels
mne.set_eeg_reference(raw, 'average', copy=False, ch_type='eeg')
Copy link
Owner

Choose a reason for hiding this comment

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

I am surprised that BLACK does not complain here. It usually formats all quotation marks as " instead of ', right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I honestly don't know, I haven't really used black formatting before! If you have a preference for double quotes over singles for this project I'll definitely stick to that.

Copy link
Owner

Choose a reason for hiding this comment

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

I mostly have a preference for using a consistent style - and preferably one that is enforced by a tool with a large community backing, such as black :-)

But if it doesn't enforce a format here, it's fine with me 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From the look of it, PyPREP doesn't have any sort of automatic linting for Black enabled other than the 88 line length, is that something that should be fixed?

And that make sense! My general habit to use single-quotes for data (e.g., variable names, argument strings, key names, etc.) and double-quotes for strings that are meant to be read as text (e.g. docstrings, print messages, exception messages, etc.), but I'm happy to conform to whatever conventions a project uses. From what I can tell the Black style guide prefers double-quotes to singles (like you thought), so up to you.

Copy link
Owner

Choose a reason for hiding this comment

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

argh, good point actually. This line is supposed to ensure in CI that all code is BLACK formatted:

But it should say black . --check instead of black . --> the former does a check, whereas the latter just applies the formatting, which is then obviously lost because it's not committed.

And for devs, the development environment should be setup automatically such that BLACK formatting is applied prior to each commit (pre-commit hook, see https://github.com/sappelhoff/pyprep#installation)

I'll make a PR for that.

Copy link
Owner

Choose a reason for hiding this comment

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

see: #85


return raw
Loading