search

lina-usc / pylossless

🧠 EEG Processing pipeline that annotates continuous data
https://pylossless.readthedocs.io/en/latest/
MIT License
24 stars 8 forks source link

Error in the use of `marks_array2flags` #18

Closed christian-oreilly closed 1 year ago

christian-oreilly commented 1 year ago

There is an error with respect to the dimensions

https://github.com/lina-usc/pylossless/blob/50f7e52f7176485ab2e028c9d900db961c226ac1/pylossless/pipeline.py#L542-L547

flag_r_ch_inds is actually an index along the epochs, whereas it should be along the channels. This create index-out-of-bound errors when an epochs with an index greater than the number of channels get flagged. I suspect that the same error is made across multiple use of marks_array2flags. I think that the use of numpy array is error prone. I suggest replacing them with XArrays which would allow to label axes and make operation less ambiguous. I also think we need to document, at a conceptual level, the different operations of the pipeline. I'll open a separate issue for this.

Unfortunately, the file that triggered that error cannot be shared openly.

scott-huberty commented 1 year ago

Thanks for this @christian-oreilly -

I'm +1 for the use of Xarray - as I recall we already use it in the methods to find bridged channels.

Do you want to work on this or do you want me to (we could also look at it during our next meeting).

christian-oreilly commented 1 year ago

Developed in the "xarray" branch.

christian-oreilly commented 1 year ago

This issue now solved on the xarray branch.