[FIX] Do not require notch frequencies to be parsed as numbers, accommodating multiples

[effigies]

While schematizing, the notch column was defined as a number. In the case of iEEG at least, the BEP leads had submitted examples where notch took the form [F1, F2, F3]. This PR recognizes this as existing practice and suggests that curators and tools adopt [F1, F2, ...] for interoperability, but does not require it.

cc @bids-standard/raw-electrophys for comment.

Closes bids-standard/bids-examples#395.

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (d56b689) 87.83% compared to head (296cebc) 87.83%.

❗ Current head 296cebc differs from pull request most recent head 4035e76. Consider uploading reports for the commit 4035e76 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1605   +/-   ##
=======================================
  Coverage   87.83%   87.83%           
=======================================
  Files          16       16           
  Lines        1356     1356           
=======================================
  Hits         1191     1191           
  Misses        165      165           

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

[adam2392]

I left some comments cuz this has been an issue for me in the past. Hopefully it helps!

[effigies]

@adam2392 Are you comfortable with the current wording leaving open the possibility of a more structured recommendation/requirement in the future?

[dorahermes]

The [low, center, high] is incorrect. The line noise is at 60Hz, so that is where you apply a notch. You sometimes also apply a notch filter to the harmonics at 120 and 180 Hz. Therefore 60Hz should not be referred to as 'low' and 120Hz is definitely not 'center'.

[effigies]

Thank you for the clarification. Is the current wording better?

[dorahermes]

Yes, thank you! Just a minor suggestion is to say: "If notch filters are applied at multiple frequencies, these frequencies MAY ..."

[sappelhoff]

I am aware that this PR is mostly to get an already existing practice finally incorporated into the spec, but it irks me that this is not very in line with:

• [ENH] Define channel column for events and Delimiter field for column descriptions #1483

Do you think we should RECOMMEND specifying the frequencies as a comma separated list like 60, 120, 240, and clarify that an old method of [60, 120, 240] is DEPRECATED?

Either way, I am approving this, because I am fine with my suggestion being dealt with in a separate PR. Thanks everyone.

[christinerogers]

@Andesha is this something you'd feel comfortable taking a moment to review?