-
Notifications
You must be signed in to change notification settings - Fork 248
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
Added support for missing channels in SpikeGadgets #1593
base: master
Are you sure you want to change the base?
Conversation
Do you have a small test file we could include. Some of these spike gadget edge cases are starting to bite us. I would love if we had some way to actually test against these edge cases so we don't break this in the future? |
I have a test file, but it's not small (27GB). Do you know how to stub a spike gadgets .rec file? |
I think @samuelgarcia knows this format a bit better. So might be worth pinging him or @alejoe91 to see if they know how. Our limit is 10MB so 27GB is definitely over haha. We will see what se can do. :) |
Actually, to test this fix we should just need the header, which should be relatively easy to stub. I'll see what I can do... |
Ok, I was able to stub the file to just include the header (no data) and now it's only 72KB. Still awaiting permission from the original researchers to share with y'all, but once I get that, I will send over the stubbed file. |
Thanks Paul, that would be awesome. This was originally caused by trying to explicitly support Intan chips which was maybe the wrong move. Previously we just gave arbitrary names to the channels which worked for everything, but was unclear to the user. If we can name things correctly both for NP and Intan that would be the best, but I don't know this system at all. Really appreciate your help on this :) |
Hey @zm711, Github doesn't support .rec as a file type, so I can't upload it here. What is the best way to share this file with you? |
The options are you can zip it and include on github or you can email it to me. Do you have a preference? |
lmk if that works |
Super busy next couple days. Will check this weekend and let you know! |
@pauladkisson, the file looks fine. I don't actually have gin on my home computer. Will submit the file to the test repo tomorrow. Then we can make sure tests pass with the new file. Then final review. |
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
@zm711 I know how to stub these files with data. Do you still have the ones from the previous issues? Let's have a call of this at some point.
We don't have the old one, but if you want to submit Paul's file to gin for me to review that would be awesome. I'm trying to find time to do it, but super busy in lab right now. |
@@ -16,6 +17,38 @@ class TestSpikeGadgetsRawIO( | |||
"spikegadgets/SpikeGadgets_test_data_2xNpix1.0_20240318_173658.rec", | |||
] | |||
|
|||
class TestSpikeGadgetsRawIOHeaderOnly(unittest.TestCase): | |||
def setUp(self): | |||
filename = "/Volumes/T7/CatalystNeuro/Jadhav/stubbed_files/SL18_D19_S01_F01_BOX_SLP_20230503_112642_stubbed.rec" |
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.
something tells me this line won't work for the CI
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.
Yes, this will need to be updated once the test file is up on the gin repo.
Fixes #1592