Extract events from spike glx

[manimoh]

Trying to fix this

[alejoe91]

Thanks @manimoh

This is great! Could you add a test for it? We have several SpikeGLX test files in the GIN epy_testing_data repo, and I'm sure (hope) at least one of them has digital events!

[manimoh]

Hi @alejoe91
Sorry for the late reply! I actually did end up looking at the SpikeGLX test files in the repo you mentioned, but none of them have digital events. I had shared a couple of files in a separate spikeinterface issue, if you could add one or both of those two the test files repo, I could write some tests using that.

[TheChymera]

@alejoe91 I pushed a test for it. Won't work right now, since there's no testing data.
Should we submit a PR for that on the gin repo directly?

[manimoh]

DigitalChannelTest_g0.zip
@TheChymera This archive are the required files that will work for the tests that we wrote

[alejoe91]

Thanks! @samuelgarcia can you upload to GIN?

[TheChymera]

@alejoe91 I submitted a PR here → https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/pulls/125

Will fix the reference as soon as that's merged.

[alejoe91]

Thanks! Can you add a line to the README (on the GIN PR) with the provenance of the file?

[TheChymera]

@alejoe91 done in https://gin.g-node.org/TheChymera/ephy_testing_data/commit/19f7708a8393b34821584922d1dc0ae499242c16

[alejoe91]

@alejoe91 done in https://gin.g-node.org/TheChymera/ephy_testing_data/commit/19f7708a8393b34821584922d1dc0ae499242c16

Merged ;)

[TheChymera]

@alejoe91 good? 5c1d2d9

Also, why is the entities_to_test variable defined but never used? 🤔

[alejoe91]

@TheChymera can you fix the conflicts? then it's should be ready!

[alejoe91]

@TheChymera acually, there seem to be somthing wrong with the files you uploaded to GIN, see https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/src/master/spikeglx/DigitalChannelTest_g0/DigitalChannelTest_g0_t0.nidq.bin

This seems to arise from a missing sync command: https://gin.g-node.org/G-Node/Info/wiki/Annexed+Content

Can you make another PR with the fix?

[alejoe91]

@TheChymera acually, there seem to be somthing wrong with the files you uploaded to GIN, see https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/src/master/spikeglx/DigitalChannelTest_g0/DigitalChannelTest_g0_t0.nidq.bin

This seems to arise from a missing sync command: https://gin.g-node.org/G-Node/Info/wiki/Annexed+Content

Can you make another PR with the fix?

@TheChymera acually, there seem to be somthing wrong with the files you uploaded to GIN, see https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/src/master/spikeglx/DigitalChannelTest_g0/DigitalChannelTest_g0_t0.nidq.bin

This seems to arise from a missing sync command: https://gin.g-node.org/G-Node/Info/wiki/Annexed+Content

Can you make another PR with the fix?

Actually tried one here: https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/pulls/127

[alejoe91]

@TheChymera the new test files were tested! But triggered some errors. Have you seen this before?

[zm711]

One of the tests was failing because of a typo (maybe a bad autocomplete? )

[alejoe91]

Ok should be fixed now :)

@TheChymera entities_to_test is indeed used by the base testing class, and that was one source of failures because "standard" tests expect some analog signals too

[TheChymera]

@alejoe91 thanks for fixing this up :) are we ready to go? ^^

[yarikoptic]

I wonder why other CI workflows didn't run., e.g.

• https://github.com/NeuralEnsemble/python-neo/blob/master/.github/workflows/core-test.yml

[alejoe91]

@yarikoptic that workflow only runs if changes in core/project.toml/workflows ;)

[zm711]

Rescaling event timestamps should always return time so I think the else comment is a good question. The idea is that the get event timestamps should return for whatever the data is in (ie frames, samples, seconds). Then if the data is not in time we use the rescale function to switch it to time. I guess my question is what is the point of using the flag use_direct_evt_timestamps?

[manimoh]

I think as I was writing that piece of code, I did wonder about the necessity of using the flag because spikeglx never actually deals with timestamps on its own, and just sample numbers. So I do think we can just safely say

event_times = event_timestamps.astype(dtype) / float(info["sampling_rate"])
return event_times

without any use of conditionals involving that flag!

[yarikoptic]

@yarikoptic that workflow only runs if changes in core/project.toml/workflows ;)

makes sense.. but why not to sweep python versions for call-iotests which are the only ones triggered here?

[zm711]

makes sense.. but why not to sweep python versions for call-iotests which are the only ones triggered here?

@yarikoptic, our iotests are currently being run from a cached conda environment to make them faster when dependencies don't change. This requires a fixed python version. If we got rid of the caching the env we could sweep versions, but then creating the new envs would take time. We actually just ran into an issue with testing where it autoupdated to python 3.12 which broke a lot of code during one of our PRs, so that would be a bit more work to improve the CI.

[zm711]

Sounds good to me. Just tagged to delete the flag and then make your edits to the rescale to simplify. @manimoh

[zm711]

We would delete this then.

[manimoh]

Done!

[zm711]

Sweet. I triggered tests. Once we run these this looks good to me. I'll let @alejoe91 have the final say since he knows spikeglx better than me.

[TheChymera]

@alejoe91 so, are we good? :3

[alejoe91]

Yep! Thanks for implementing this @manimoh @TheChymera !!