Allow adding `pipeline.flaggedChs `to `raw.info['bads']`

[scott-huberty]

I imagine there is going to be disagreement on this so I want to get the conversation started:

If I run the code in test/test_simulated.py I get this:

• In the automated pipeline we flag time periods as annotations starting with _bad. Annotations are saved directly in the file when exporting to .edf.
• We do not add flagged channels to raw.info['bads'], we only save them to pipeline.FlaggedChs

This is inconsistent and confusing. Right now the user will see some of the pipelines decision in their file and need to add extra boilerplate to see the flagged channels.

Also QCGUI does nothing with flaggedChs after loading, and MNEVisualizer only knows about raw.info['bads'], so the dashboard doesnt show the pipelines decisions on channels right now and if the user clicks on a channel, it is added to raw.info['bads'], not flaggedChs.

I think the closer we integrate with MNE here the better. I think the FlaggedChs.tsv file is necessary but lets not make everyone life harder with more boilerplate just to see decisions on their data that aren't even destructive, for the sake of purity.

[christian-oreilly]

I think these things should remain separate (for various reasons including the philosophy behind lossless and the fact that there is a degeneracy when we go from FlaggedChs to info["bads"]) but we should provide utilities to help to make the bridge (i.e., something like raw = mark_bad_channels(raw, kinds=["eog", "ch_sd"])). Also, I think that this issue is rehashing discussion we already had (e.g., with respect of how we distinguished "bads" in MNE vs as automatically marked by the pipeline vs manually marked as bad by the user). This also relates somehow to using HED tags (issue #70; i.e., how we want to carry information of a full annotation as opposed to a binary classification).

[scott-huberty]

we should provide utilities to help to make the bridge

I was also thinking we could add a kwarg to run like add_bads that the user can flip on to add the flagged_chs to bads and to prepend _bad to annotations. saves them the extra step and I think many users would like this (even I wished I had this while testing). It also makes our treatment of flagged_epochs and flagged_chs more consistent.

I don't think any info is lost. The reason for it being flagged is still in flaggedChs.

Also +1 for a helper function.

[christian-oreilly]

The add_bad arguments would just trigger the execution of the mark_bad_channels(...) at the end of the pipeline. I'd make it either a boolean or a list of strings or a dictionary False: Nothing gets marked as bad True: Everythin gets marked as bad ["ch_sd"] : Mark as bad everything labeled with "ch_sd", regardless of whether it is for channels, epochs, of ICS {"epochs": ["ch_sd"]} : Mark as bad every "epochs" labeld with "ch_sd" (not epochs really, but time windows). This input could be passed directly to the kinds argument of the mark_bad_channels() function.

[scott-huberty]

Yeah I think this is a nice solution. Glad we could find some common ground 😉

[Andesha]

is there any updates to this now that flagging has changed formats slightly? @scott-huberty

[scott-huberty]

is there any updates to this now that flagging has changed formats slightly? @scott-huberty

So We landed on following lossless's historical step of requiring a deliberate purge of annotations.

But practically speaking, so far all user (us) just want to purge the annotations.

So We need to add a helper function to the pipeline to convert flags to bads, and make the default behavior of the pipeline to call this function at the end of the run, that a user could set to False to disable.

The routine would be Something like:

flagged_chs=pipeline.flags['ch'].copy()

flagged_chs.pop('rank')
Pipeline.raw.info['bads'].extend(flagged_chs.values())

# ditto for ICs 
...

I started the code for this on another branch. I could push it up this week hopefully?