mne-python
mne-python copied to clipboard
fix prefilter management for EDF/BDF
What does this implement/fix?
For EDF, BDF, and GDF formats, prefilter information, including highpass and lowpass filters, is stored in the info structure. Arrays for highpass/lowpass are generated by the _parse_prefilter_string function, which omits adding a value if no filter information is found.
These arrays are constructed using all channels except the last one, which is presumed to be an Annotation or TAL channel.
I think it should use only selected channels.
Furthermore, it is occasionally necessary to access highpass/lowpass values in _raw_extras for corresponding channel information. To address this, I suggest the following modifications:
- The method to create prefiltering values is now adjusted to include only selected channels, using [sel].
- The
_parse_prefilter_stringfunction has been improved to insert an empty string "" when no information is available. It has also been enhanced to parse strings like "DC".
I have reviewed filter string formats for Biosemi EEG ECG EMG BSPM NEURO amplifiers systems, EDF specification, EDF+ specification.
In _get_info, where highpass/lowpass values are assigned to info, the current code exhibits bugs:
- The condition
all(highpass)is always true since highpass and lowpass are arrays of non-empty strings. The correct approach to check if all values are the same should be likenp.all(highpass == highpass[0]). - The assignment
info["highpass"] = float(np.max(highpass))fails because highpass contains strings. However, due to the always-true nature ofall(highpass), this code is never executed.
This PR addresses and fixes these issues as well. With these changes, some EDF files will trigger warnings about varying highpass and lowpass filter settings across channels, which, due to a bug, were previously not displayed. These warnings are essential for ensuring data consistency and integrity.
@larsoner Thanks for the reply. Yes, it seems several tasks need to be updated which I did not checked locally. I will update with local tests for these jobs.
Yes, I can take a look early next week!
@cbrnr you happy with this now? If so we could get it into 1.7 I think
It's been some time since I commented on this PR. Could you or @rcmdnk summarize the public (user-facing) changes? Are there any? "Fix pre-filtering management" doesn't really tell me a lot about the changes (and this more detailed information should go into the changelog entry as well). Thanks!
@cbrnr The problem is that the current implementation inaccurately handles the highpass and lowpass prefilter information in the Raw.info.
Bugs:
- All prefilter information other than empty channels are stored in _raw_extras
- no selection by selected channels, empty channels are ignored.
- all(highpass) seems to be used to check uniformity of highpass values across channels.
- However, it fails to trigger the warning for differing highpass filters because it incorrectly checks for all True values instead of uniformity.
- info["highpass"] = float(np.max(highpass)) does not work because highpass is an array of strings. (but this does not occurs due to the previous
allcheck.
Changes Introduced in This PR:
- The info['highpass'] and info['lowpass'] will now correctly reflect the maximum and minimum prefilter values from the selected channels, respectively.
- Remain all channel values in _raw_extras
- Put empty string
""if no the information is empty. - Select values for selected channels in
_get_infoto make highpass/lowpass of Raw.info
- Put empty string
- Warn if selected channels have different prefiltering values.
In it goes – the changelog entry is OK, people can always refer to this thread for more details. Thanks @rcmdnk!