search

NeuralEnsemble / python-neo

Neo is a package for representing electrophysiology data in Python, together with support for reading a wide range of neurophysiology file formats
http://neo.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
322 stars 247 forks source link

An OSError about header when importing neuralynx data #1202

Open redagavin opened 1 year ago

redagavin commented 1 year ago

Describe the bug I was trying to import neuralynx data into python using neo. But, I got an OSError. This is a screenshot of the error. Screen Shot 2022-11-27 at 5 07 43 PM

Also, I noticed that there is a small bug in the line that raise the OSError. I think .format(an, av) is supposed to fill the first {} with an and the second {} with av. However, only the second string after the + calls .format(an, av) in the source code, which is why the error message is weird.

Screen Shot 2022-11-27 at 5 19 11 PM

To Reproduce In order to reproduce, just use the two lines of code in the screenshot. The error occurs when reading a file that I can't share publicly. Please get in touch with me and I will share the file privately.

Environment:

redagavin commented 1 year ago

@samuelgarcia @JuliaSprenger @apdavison Could you please take a look at this? Thanks!

samuelgarcia commented 1 year ago

Hi, thanks for the report. Could you send us a file to debug this ?

@JuliaSprenger @PeterNSteinmetz: would you have time to handle this, you are our best neuralynx expert!

redagavin commented 1 year ago

Hi, thanks for the report. Could you send us a file to debug this ?

@JuliaSprenger @PeterNSteinmetz: would you have time to handle this, you are our best neuralynx expert!

@samuelgarcia @JuliaSprenger @PeterNSteinmetz Yes, I can send you a file to debug this. Please provide me with a link so that I can share the file privately.

PeterNSteinmetz commented 1 year ago

Gavin, I can take a look at that. Can you send via email to PeterNSteinmetz@steinmetz.org ? Alternatively, do you use Google Drive? If so you could share with me there - username is PeterNSteinmetz

redagavin commented 1 year ago

Gavin, I can take a look at that. Can you send via email to PeterNSteinmetz@steinmetz.org ? Alternatively, do you use Google Drive? If so you could share with me there - username is PeterNSteinmetz

@PeterNSteinmetz I sent the files to you through email and google drive. Please take a look. Thanks!

samuelgarcia commented 1 year ago

Thank you Peter!

PeterNSteinmetz commented 1 year ago

I now have a fix for this issue. New undocumented header strings for Cheetah v 5.6.0 caused this. I want to add the test data to ephy_testing_data.

So @redagavin, can you please provide permission and desired credit/attribution for including one shortened .ncs file (26.8 kB) and one shortened .ntt file (22.4 kB) from what you sent me to the repository at https://gin.g-node.org/NeuralEnsemble/ephy_testing_data

PeterNSteinmetz commented 1 year ago

@samuelgarcia @JuliaSprenger Also will need some help here again placing the data in the ephy_testing_data repository. (Sorry but this whole gin thing is not very clear to me because I usually think in git terms).

I have my own clone of this project and have that checked out and synced and have a master branch on the web site. What do I need to do to make sure that my master is up to date with the project master? I don't see any sort of sync button.

Then I need to create a branch, unlock the Neuralynx folder, add a new sub-folder with two files, lock it and then commit and push to my branch on my repository. Then open a pull request on the project master?

Is there somewhere where this sort of adding things to an already existing clone is laid out?

PeterNSteinmetz commented 1 year ago

@samuelgarcia @JuliaSprenger We have no word yet on the use of the sample files for testing. How do you want to proceed?

I could just create a code branch for Neo with the fixes but without any tests.

Alternately, I could hack the headers on these a bit to anonymize them and either remove all the data records or replace the data with random samples. The test files would then be completely synthetic, but not as realistic.

PeterNSteinmetz commented 1 year ago

@samuelgarcia @JuliaSprenger Bumping this as I was just going to add something to NeuralynxRawIO and want to be up to date with the main branch.

JuliaSprenger commented 1 year ago

I have my own clone of this project and have that checked out and synced and have a master branch on the web site. What do I need to do to make sure that my master is up to date with the project master? I don't see any sort of sync button.

If you are using the gin-cli simply gin download will update your local copy of the branch you have currently checked out.

Then I need to create a branch, unlock the Neuralynx folder, add a new sub-folder with two files, lock it and then commit and push to my branch on my repository. Then open a pull request on the project master?

For adding new files you don't need to unlock existing files (folders can not be in a locked state, only files can). Also note that since git annexed files are not shared between forks, you should push your feature branch directly to the Neuralensemble/ephy_testing_data repo and not your fork. You should have permission to do so as you are already listed as a collaborator. To be sure the gin-cli runs smoothly ensure that you are using a git-annex<9 and not the latest version, since we noticed some issues with that version (see also this issue)

Tell me if you have further question following the contribution guideline

PeterNSteinmetz commented 1 year ago

@JuliaSprenger Actually there is the issue noted here in the Jan 23 comment. We don't have permission to use the test files which were given.

JuliaSprenger commented 1 year ago

@redagavin Could you comment on this?

So @redagavin, can you please provide permission and desired credit/attribution for including one shortened .ncs file (26.8 kB) and one shortened .ntt file (22.4 kB) from what you sent me to the repository at https://gin.g-node.org/NeuralEnsemble/ephy_testing_data

PeterNSteinmetz commented 1 year ago

Well see if that produces anything. I have been in touch with him via email to get the files and wrote and received nothing.

GeezleCode commented 1 year ago

Hi, I solved a similar bug in my fork. The issue was that neo didn't recognize the "RawDataFile" option in the headerstring. This is set when ".ncs" files are created by offline replay of raw data ".nrd" files. You'll find the fork here https://github.com/GeezleCode/python-neo. The branch containing the few changes is called "NeuralynxIORawDataSystem". I haven't created a pull request yet.

JuliaSprenger commented 1 year ago

Hi @GeezleCode Thanks for fixing another issue of the NeuralynxIO. If you want your fix to be included in neo, could you open a PR?

GeezleCode commented 1 year ago

@JuliaSprenger Done! #1234

JuliaSprenger commented 1 year ago

@redagavin Is this resolved for you? Could you comment on https://github.com/NeuralEnsemble/python-neo/issues/1202#issuecomment-1466565212 ?

PeterNSteinmetz commented 1 year ago

@JuliaSprenger I have the code which fixes this issue but the test data has not been added as we don't have permission. I can add it without test data if you prefer.

From my look at #1234, I think that will not resolve this issue as this issue has to do with date formatting being version sensitive.

JuliaSprenger commented 1 year ago

@PeterNSteinmetz If it's just a small change we could add it anyway. Without test files we can just not guarantee that it will not reoccur at some point in the future...

PeterNSteinmetz commented 1 year ago

Pull request 1257 should handle this case now.