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

New OpenEphys File Naming Convention Breaking openephysio #1320

Closed
stef1029 opened this issue Aug 31, 2023 · 25 comments · Fixed by #1453
Closed

New OpenEphys File Naming Convention Breaking openephysio #1320

stef1029 opened this issue Aug 31, 2023 · 25 comments · Fixed by #1453
Assignees
Labels
Milestone

Comments

@stef1029
Copy link

stef1029 commented Aug 31, 2023

Loading the files saved within an OpenEphys format recording node file seems to cause an issue, unless I'm trying to load the wrong directory.
data = neo.OpenEphysIO(str(dirname))
File "C:\Users\Stefan R Coltman.conda\envs\BehaviourControl\lib\site-packages\neo\io\openephysio.py", line 11, in init
BaseFromRaw.init(self, dirname)
File "C:\Users\Stefan R Coltman.conda\envs\BehaviourControl\lib\site-packages\neo\io\basefromrawio.py", line 74, in init
self.parse_header()
File "C:\Users\Stefan R Coltman.conda\envs\BehaviourControl\lib\site-packages\neo\rawio\baserawio.py", line 179, in parse_header
self._parse_header()
File "C:\Users\Stefan R Coltman.conda\envs\BehaviourControl\lib\site-packages\neo\rawio\openephysrawio.py", line 78, in _parse_header
info = self._info = explore_folder(self.dirname)
File "C:\Users\Stefan R Coltman.conda\envs\BehaviourControl\lib\site-packages\neo\rawio\openephysrawio.py", line 481, in explore_folder
seg_index = int(s[2]) - 1
ValueError: invalid literal for int() with base 10: 'CH1'

This happens when dirname is set to C:\ .... \Open Ephys\2023-08-31_10-48-18\Record Node 118, which contains the .continuous files from the recording.

Environment:

  • OS: Windows 11
  • Python 3.7
  • neo 0.12.0
@stef1029 stef1029 added the bug label Aug 31, 2023
@zm711
Copy link
Collaborator

zm711 commented Aug 31, 2023

@stef1029
Just for completeness could you put the version of Neo you are currently using and the names of the *.continuous files you have (just one or two). Neo is expecting the file to be of a format like 100_CH0.continuous etc. Is yours like that or are the labels different?

@apdavison apdavison added this to the 0.14.0 milestone Sep 1, 2023
@stef1029
Copy link
Author

stef1029 commented Sep 5, 2023

@zm711
I've updated the environment details with the neo version. I've also tried running it in python 3.11.4, since I saw on the pypi page that neo needs python 3.8+, however am still having no luck. My file names seem to be in the right format, as "117_RhythmData_CH1.continuous" as an example.

@zm711
Copy link
Collaborator

zm711 commented Sep 5, 2023

@stef1029

Thanks for the file example. One more question. Did you add the RhythmData? Or did it come like that?

Basically Neo is expecting the exact format I sent so number_channel.continuous whereas your file has number_name_channel.continuous. If this is a change then this is a Neo bug that needs to be fixed. If you added the name RhythmData then I would say try deleting that since Neo doesn't currently support the format. And then this would be a feature request rather than a bug.

@stef1029
Copy link
Author

stef1029 commented Sep 5, 2023

@zm711
As far as I can tell the RhythmData part of the name has been added by the acquisition software. I'm using a plugin for the OE acquisition board within the OE acquisition GUI. Within the docs for the OE format it seems like the name of the .continuous file is meant to have some component in there, which I'm guessing varies with the plugin you use.

@stef1029
Copy link
Author

stef1029 commented Sep 5, 2023

@zm711
Copy link
Collaborator

zm711 commented Sep 5, 2023

@stef1029

Thanks. Yep that is a change from when the original reader was written. Sorry about that!
That will likely be a larger change than a bug fix since this will require a bit of a rewrite or a new reader. What is the RhythmData? Neo is currently focused on ephys with some events logic. Would the rhythm data be in scope for Neo?

@stef1029
Copy link
Author

stef1029 commented Sep 5, 2023

@zm711
Yep it's exactly that as far as I know, I believe it's called that after some of the firmware in the acquisition board. Thanks for the help.

@zm711 zm711 closed this as completed Feb 19, 2024
@NilpawanRC
Copy link

@stef1029
Hi, I am also facing the same problem, " ValueError: invalid literal for int() with base 10: 'CH1' ", with the .continuous file having name like "124_RhythmData-A_CH1.continuous". Could you please elaborate a little how you solved the issue?

@stef1029
Copy link
Author

@NilpawanRC
Hi, after this issue I didn't end up using Neo to access the data in the end, and instead used the Open Ephys Python tools instead, which are their specific tools for doing analysis.

@zm711
Copy link
Collaborator

zm711 commented Mar 25, 2024

Just to be clear neo is for ephys data specifically. If your ephys data is also stored in the same file we can try to figure out how to extract it. If the rhythmdata is something completely different it might be hard to fit it within neo. It could be that you just need to remove that file from your folder to get your ephys data.

@NilpawanRC
Copy link

@stef1029
Okay, thank you. I will also go for that then.

@NilpawanRC
Copy link

@zm711
Actually the old Open Ephys GUI used the format like "100_CH0.continuous" to save the Ephys data. But the new Open Ephys GUI changed it completely and now it save the Ephys data as "124_RhythmData-A_CH1.continuous". And this new format is also mentioned in their website : https://open-ephys.github.io/gui-docs/User-Manual/Recording-data/Open-Ephys-format.html

@zm711
Copy link
Collaborator

zm711 commented Mar 26, 2024

@NilpawanRC, that was actually my original question. If this is the new naming of open ephys (ie all data now has this naming convention) then we would absolutely want to add it (we would just need you to share a dataset with us). If this is a different type of data (like behavioral data) because of a plug-in then it might not fit with this codebase. Does that make sense?

@NilpawanRC
Copy link

Yes, the regular, continuous OpenEphys ephys signals are now called something like xxx_RhythmData_CHx.continuous. They already came from via Rhythm plugin (for OpenEphys Intan boards), but now this is also visible in the filename.
Maybe this is because the newer OEphys software can also record from Neuropixels. I believe some of the newer acquisition boards also use a different plugin with possibly a different name (I haven't tried that out).

@zm711
Copy link
Collaborator

zm711 commented Mar 26, 2024

If that's the case would you be willing to share a (tiny) file that we could debug on and update the code?

@zm711 zm711 reopened this Mar 26, 2024
@zm711 zm711 changed the title OpenEphysIO issue New OpenEphys File Naming Convention Breaking openephysio Mar 26, 2024
@NilpawanRC
Copy link

@zm711
Copy link
Collaborator

zm711 commented Mar 26, 2024

Cool thanks. I'm a bit busy for the next couple days, but I will check it out soon. Feel free to re-ping this issue :)

@NilpawanRC
Copy link

Yeah sure, thank you.

@zm711
Copy link
Collaborator

zm711 commented Apr 1, 2024

@NilpawanRC,

I've almost got a working copy for you to test, but I am getting that the events file is empty. Is that the case? If it's not I'll need some more time to figure out what is going on. But if it is empty I'll post a PR for you to test this week.

@NilpawanRC
Copy link

Yeah I have created any events during recordings. That should be empty. You can share with me to test. Thank you so much.

@zm711
Copy link
Collaborator

zm711 commented Apr 2, 2024

@NilpawanRC, so we have one other openephy PR open that will create merge conflicts with mine (so I'll need to fix that before we can accept the PR on our side). Would it be possible for you to just turn on the recording equipment for 1-2 secs and send us a dummy Node to add to our test suite. We really prefer having test files before we accept a PR changing a reader. (Size limit is really <20MB but ideally ~10MB).

@zm711
Copy link
Collaborator

zm711 commented Apr 2, 2024

Sorry one last thing that I need to know. What is 124_RhythmData-A.events then? In our code we assume that events data will be in something called all_channels.events. Should I ignore this file? It is causing my testing to break which is why I asked about the event data....

@NilpawanRC
Copy link

yes in old format it was "all_channels.events" but now it changes to "XXX_RhythmData-A.events".

And yes I can surely record for 1-2 secs and send the file to you.

@NilpawanRC
Copy link

I have uploaded the dummy node file in the previous link.

https://drive.google.com/drive/folders/1w-r2xyerlkz0w4TftvRlqsbDQKdnD9O9?usp=sharing

@zm711
Copy link
Collaborator

zm711 commented Apr 2, 2024

I have linked the PR. Once it finishes testing with our old files then you are free to test it by installing neo from that PR.

@zm711 zm711 self-assigned this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants