Fix/edf d discontinuous detection #13583
Open
+434
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Detect and raise NotImplementedError when loading EDF+D or BDF+D files that contain actual gaps between data records, instead of silently treating them as continuous data. This prevents incorrect time alignment. Fixes mne-tools#13429
- Validate n_records matches len(record_times), raise ValueError on mismatch - Validate record_times are numeric (no strings or invalid types) - Check for non-finite values (NaN, inf) and raise ValueError - Warn and sort if record_times are not monotonically increasing - Add comprehensive tests for all validation scenarios
Arnav1709
requested review from
agramfort,
dengemann,
drammock and
larsoner
as code owners
|
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference issue (if any)
Fixes #13429.
What does this implement/fix?
This PR addresses a critical bug where MNE-Python would silently load EDF+D (discontinuous) and BDF+D files with acquisition gaps between data records, treating them as continuous data. This could lead to:
RawEDF.timesarray would show continuous timestamps, ignoring gapsChanges made:
Detection of EDF+D/BDF+D files: Modified
_read_edf_header()to read and check the 44-byte reserved header field forEDF+DorBDF+Dmarkers.Gap detection: Added two helper functions:
_get_tal_record_times(): Extracts onset times of each data record from TAL (Time-stamped Annotation List) annotations_check_edf_discontinuity(): Compares expected vs. actual record timestamps to detect gapsError handling: Modified both
RawEDFandRawBDFclasses to check for discontinuities after reading TAL data. When gaps are detected, aNotImplementedErroris raised with:Backward compatibility: EDF+D/BDF+D files that are marked as discontinuous but have no actual gaps (e.g., from some Nihon Kohden systems) continue to load normally.
Documentation: Updated docstrings for
read_raw_edf()andread_raw_bdf()to document this limitation.Tests: Added comprehensive tests covering:
Additional information
This is a breaking change in behavior: files that previously loaded (incorrectly) will now raise an error. However, this is intentional as the previous behavior was incorrect and could lead to data analysis errors.
The error message provides actionable guidance, suggesting specialized tools (luna/lunapi) for handling discontinuous files or converting to continuous format if gaps are not significant.
The implementation follows the EDF+ specification for detecting discontinuous files and uses TAL annotations (which are required in EDF+ files) to detect actual gaps.
Related discussion in the GitHub issue suggests that full support for EDF+D would require filling gaps with NaN data (similar to how Eyelink handles gaps), which is a more complex feature that could be implemented in a future PR.