Reference interpolates instead of deletes channels manually marked as bad, closes #146

[john-veillette]

PR Description

Fixes #146, an error that results from the raw.pick_types call inside pyprep.Reference deleting channels which have been marked as bad prior to PREP. These channels should now be interpolated instead.

A new field 'bad_manual' is added to noisy_channels_before_interpolation dict so there's still a record the channel was interpolated. This happens inside Reference; the NoisyChannels class is untouched.

Merge Checklist

• the PR has been reviewed and all comments are resolved
• all CI checks pass
• (if applicable): the PR description includes the phrase closes #<issue-number> to automatically close an issue
• (if applicable): the changes are documented in the changelog changelog.rst
• (if applicable): new contributors have added themselves to the authors list in the CITATION.cff file

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.60%. Comparing base (06fb37b) to head (a6f8d10).
Report is 8 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #156   +/-   ##
=======================================
  Coverage   98.59%   98.60%           
=======================================
  Files           7        7           
  Lines         713      717    +4     
=======================================
+ Hits          703      707    +4     
  Misses         10       10           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sappelhoff]

Thanks! Could you please make a small modification to an existing test to also cover this behavior?

Also, when would "bad_manual": ..., now be printed, and when would it not be printed?

For example, it won't come up in

pyprep/pyprep/find_noisy_channels.py

Line 17 in 48b7349

class NoisyChannels:

, right?

[john-veillette]

Sure, I can take a look at the tests, probably on Monday.

Re: "bad_manual", I think it only prints when logger.info(f"Bad channels: {self.noisy_channels_original}") is called in Reference.robust_reference. So when you run the full PREP pipeline, it won't show up with the first NoisyChannels instance but will during re-referencing steps. (Not sure what that noisy_detector = NoisyChannels is actually doing in prep_pipeline.py, come to think of it, it seems like the instance is never referenced again.)

Internally, NoisyChannels ends up ignoring manually marked bad channels due to the mne.Raw.pick_types call on line 62, so nothing needed to be changed there.

[sappelhoff]

Not sure what that noisy_detector = NoisyChannels is actually doing in prep_pipeline.py, come to think of it, it seems like the instance is never referenced again

Thanks for pointing this out, this is really dead code:

• remove dead code from fit method #157

Internally, NoisyChannels ends up ignoring manually marked bad channels due to the mne.Raw.pick_types call on line 62, so nothing needed to be changed there.

Thanks for looking into this, I just had a look as well and I had forgotten about this. It would be nice to have the same logic in NoisyChannels, that is, report about channels in raw.info["bads"] at initialization with a bad_manual entry in the dict one can obtain through get_bads ... and handle those channels the same way as those in "bad_by_flat_nan". I think that'd be an improvement to transparency compared to simply dropping them, and the practical outcome is the same.

Furthermore if we implemented this in NoisyChannels directly, then robust_reference would also get this information through these lines:

pyprep/pyprep/reference.py

Lines 212 to 218 in 48b7349

	noisy_detector = NoisyChannels(
	raw,
	do_detrend=False,
	random_state=self.random_state,
	matlab_strict=self.matlab_strict,
	)
	noisy_detector.find_all_bads(**self.ransac_settings)

and your main goal would also be achieved (being able to pass a raw object with bad channels already marked without them being dropped).

I am sorry that this comment changes the scope of this PR somewhat, but I hadn't looked into the issue deeply enough to understand all implications before you got started.

[john-veillette]

Okay, I've added a "bad_by_manual" entry to the output of NoisyChannels.get_bads(as_dict = True), and some test coverage for this behavior to test_find_noisy_channels.py. (I changed the name from bad_manual to bad_by_manual since some of the internal logic of get_bads depends on the bad_by_ prefix.)

Now "bad_by_manual": ... should be printed anytime the logger prints bad channels.

[sappelhoff]

Thanks a lot @john-veillette!