mark_channels behavior unexpected/unintuitive

[drammock]

Yesterday I did something like mark_channels(bids_path=my_bp, channels=[], status="bad"). I was expecting it to be a no-op (because channels is an empty list) but in fact it marked all channels as bad. Now, the docstring is clear on this point, so I can't really complain about that. But I would argue that it is really unintuitive that passing a list of 1 channel name marks 1 channel, but a list of 0 channel names marks all channels (instead of zero channels).

My use case: we have prebad.txt files listing which channels were noted as noisy during the recording. Sometimes those files are empty. It's useful to have that file exist and be empty, rather than just not existing at all, because then you know that someone looked for bad channels and found none (if "no bad channels" was indicated by absence of the file, you wouldn't know whether someone checked and found no bad channels, or just forgot to check at all).

In that context, my script finds the prebad file, reads it, and if there are no channel names in that file, the prebads variable (which I pass to mark_channels()) is an empty list --- which does not do what I want it to!

My suggestion is that if you want to have a signal value that will mark all channels as good/bad, that signal value should be something like "all" rather than []. If you agree, feel free to assign this issue to me and I can implement the change.

[hoechenberger]

I agree that passing an empty list should be a no-up or raise an exception

the default could be changed to the "all" value you're proposing

[drammock]

the default could be changed to the "all" value you're proposing

Currently there is no default (it's a required kwarg), so nothing needs to change there. But since the behavior of channels=[] would be changing, there would need to be a deprecation cycle.

[sappelhoff]

+1 for the proposed change and a deprecation cycle.