Skip to content
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

Switch to EDF.jl #91

Open
rob-luke opened this issue Aug 18, 2021 · 12 comments
Open

Switch to EDF.jl #91

rob-luke opened this issue Aug 18, 2021 · 12 comments

Comments

@rob-luke
Copy link
Owner

The current reader used is https://github.com/sam81/BDF.jl which hasn't been updated since 2018, this is only an issue as it is requiring old package dependencies.

Best plan seems to be to switch over to beacon-biosignals/EDF.jl#46 once its merged.

@rob-luke
Copy link
Owner Author

Or fork and maintain BDF.jl, but that's less than ideal

@rob-luke rob-luke moved this from Blocked to In Progress in Revitalise Neuroimaging.jl Sep 14, 2021
@ararslan
Copy link

ararslan commented Oct 2, 2021

FYI since you linked the PR, EDF.jl v0.6.3 supports reading and writing BDF files 🙂

@rob-luke
Copy link
Owner Author

rob-luke commented Oct 2, 2021

That's amazing! Thanks so much for your hard work @ararslan. I will switch to your package when I have a spare few hours. I appreciate your contribution greatly!

@rob-luke rob-luke changed the title Add new BDF reader Switch to EDF.jl Oct 2, 2021
@rob-luke rob-luke removed the warnings label Oct 2, 2021
@rob-luke
Copy link
Owner Author

I played with this a few weeks ago and imported data with no issue, however, I was unable to extract annotation information. This seems to have been because I was reading old documentation, but is now fixed in beacon-biosignals/EDF.jl#52. So I should try and extract annotation information again.

Once this is working I would like to refactor the BDF reading code as there is lots of code repetition. And I would like to make the read functions more agnostic of the datatype being read.

@likanzhan
Copy link
Contributor

If I understand correctly, it seems that the status channel in BDF cannot be correctly handled by EDF.jl for now.

@rob-luke
Copy link
Owner Author

rob-luke commented Nov 1, 2021

If I understand correctly, it seems that the status channel in BDF cannot be correctly handled by EDF.jl for now.

Oh no. That's bad news. We need the status channel for BDF.

What about for BDF+? Does EDF.jl correctly extract triggers for BDF+ files?

@likanzhan
Copy link
Contributor

The BDF+ and EDF+ trigger information specified as Annotations can be handled. But the BDF status channel specified by biosemi is not correctly handled for now.

@rob-luke
Copy link
Owner Author

rob-luke commented Nov 1, 2021

Thanks for the summary @likanzhan! So I will continue with my plan to refactor the current reading code to be less BDF specific (remove hard coding of parameters, remove repeated code, modularise tasks). Then we can add EDF.jl to enable reading of EDF and BDF+ files. And use BDF.jl and EDF.jl concurrently.

I will report back here when its done. But feel free to try and tackle it in the meantime (its just that part of the code is pretty BDF specific, sorry)

@ararslan
Copy link

ararslan commented Nov 1, 2021

@likanzhan, can you open an issue on EDF.jl with some details about the status channel not working? Without supporting that, BDF support is technically incomplete, so I'll see if I can get that fixed. I don't have access to any BDF files with a status channel though.

@likanzhan
Copy link
Contributor

likanzhan commented Nov 26, 2021

Hi @rob-luke, when refactoring the reading code, is it possible to use the format being described as Onda.jl?

@likanzhan
Copy link
Contributor

Hi @rob-luke,

The branch beacon-biosignals/EDF.jl#61 (not released yet) can now handle the status channel.

@ararslan
Copy link

Apologies, I've not been able to get back to that PR for a while. One question I have for the folks who would be using it is what behavior would be desirable for calling EDF.decode on the trigger status channel. It could decode the individual bits, as BioSemi's description mentions that information is encoded via bit masks. I lack sufficient context to know what makes sense for the trigger status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants