-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MISC] make explicit that EDF+ (and for EEG: BDF+) are included in iEEG / EEG format requirements #831
Conversation
This seems reasonable to me, maybe we should gather an EDF+ and a BDF+ file and add them to mne-bids in a test though. Doing that first might be helpful to know if there are any issues but I think the issues will be able to be solved anyway so whichever order really. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I just did this: MNE-BIDS tests, we are using this EDF file, which is EDF+ as one can verify by reading it via https://gitlab.com/Teuniz/EDFlib-Python To check for BDF+, I generated a test file (also via EDFlib-Python) and read it via MNE-Python's I will attach test files here as a zip for other people to check out. The Zip contains a README and tiny edf, edf+, bdf, and bdf+ files. The EDF and BDF files were created using https://github.com/holgern/pyedflib, because EDFlib-Python does only support writing to EDF+ and BDF+. (Not sure why, maybe to nudge people towards using the newer standard). EDIT: Actually, there are also test files (EDF/EDF+/BDF/BDF+) here: https://www.teuniz.net/edf_bdf_testfiles/ One additional thing: @Teuniz - Perhaps you can offer your EDF(+)/BDF(+) expert opinion here. This is the question:
In the two questions above I assume that the "readers" I refer to are implemented correctly, according to the EDF(+)/BDF(+) specification(s). Your input would be very much appreciated! |
Yes, that's correct. |
okay, thanks everyone. I'll leave this PR open for a bit longer so other people have the option to chime in if they want. But basically, this is ready to be merged. Out of curiosity @Aaronearlerichardson --> What software do you use to read and write your BDF+ files? Have you ever come across incompatibilities with libraries such as FieldTrip, EEGLAB, or MNE-Python? I think with regards to BDF+ it's possible that several packages out there do not yet support it, and thus reading BDF+ with these packages will not provide you with the annotations and "enriched patient data" that comes with the "+" of BDF+. Is that what you experience? In terms of BIDS that wouldn't be a problem of course, because that information is in the BIDS metadata files. |
closes #821
As proposed by @Aaronearlerichardson I here make explicit in the spec, that the EDF and BDF formats supported in EEG and iEEG also include EDF+ and BDF+.
Both EDF+ and BDF+ are backward compatible with EDF and BDF tools, the file extensions are the same, and technical, open accessible specifications of the formats are available and linked.
@CPernet and @robertoostenveld were not particularly enthusiastic, but also not against this change.
I personally deem this change more of a clarification and I assume that most of the BIDS
.edf
files that are "around" these days are actually EDF+ files anyhow (just my assumption though, I didn't check).