python-neo icon indicating copy to clipboard operation
python-neo copied to clipboard

Fix electrode selection in Axona raw recording

Open letiziasignorelli opened this issue 1 year ago • 6 comments

I'm proposing this PR to fix #1454

@zm711

letiziasignorelli avatar Aug 05 '24 09:08 letiziasignorelli

@alejoe91 do you know this datalad error? it only happened for one dataset on the 3.9 python vs the 3.11 python runs. It's completely unrelated to this PR, but if you recognize the issue then I won't spend time digging into it.

zm711 avatar Aug 05 '24 11:08 zm711

@zm711 maybe just a random failure, re-triggered to see what happens!

alejoe91 avatar Aug 05 '24 13:08 alejoe91

I tried retriggering once, but yeah let's see if maybe it was just a timing issue.

EDIT; --Yep okay just a random ci failure. cool cool

zm711 avatar Aug 05 '24 13:08 zm711

Gin data for this? : O

h-mayorquin avatar Aug 23 '24 18:08 h-mayorquin

Gin data for this? : O

You mean you want extra gin data? This was a bug in the original implementation just because we didn't know which channel was which. If you use the current gin data with the spikeinterface extractor you'll see that the channels are just 1-16 or 1-32 rather than being meaningful.

zm711 avatar Aug 23 '24 19:08 zm711

Thanks. I did not realize we already had the gin data as I came from the neuroconv trail.

h-mayorquin avatar Aug 26 '24 15:08 h-mayorquin

@letiziasignorelli just wanted to see if you'd have bandwidth to work on this before the end of the year :) ? Otherwise I'll change the tag to future for now so that we just let you get back to it when you want....

zm711 avatar Dec 02 '24 13:12 zm711

@zm711 I replied to all comments, and from my side I was just waiting for approval :). Is there anything I should still work on/change?

letiziasignorelli avatar Dec 02 '24 16:12 letiziasignorelli

Did you forget to push the changes? I don't see any of the changes--(I see your comments, but not a commit where you have the changes). Once they are pushed here then the last thing is just resolving the conflict in the author doc :)

zm711 avatar Dec 02 '24 16:12 zm711

Okok you're right, I'll push the changes in the next few days. Thanks :)

letiziasignorelli avatar Dec 02 '24 16:12 letiziasignorelli

Hi @letiziasignorelli. Thanks for this. Sorry the ultra lon delay. I will read it more carrfully, I am not sure to follow entirely the logic. Give me another read.

samuelgarcia avatar Jan 06 '25 14:01 samuelgarcia

@samuelgarcia just so you know the background (although I really appreciate the extra read :))

In our original implementation we were just giving the channels arbitrary names like range(0,32) when in fact the channels have actual names depending on which tetrode they are (so it should be something like A1, A2, A3, A4, B1, B2, B3, B4 instead of 0,1,2,3,4,5,6,7,8.

But I'm just trying to make sure the slicing of channels when some aren't turned on will be correct.

zm711 avatar Jan 06 '25 14:01 zm711

Maybe we should merge this? It's been six months.

h-mayorquin avatar Jun 18 '25 21:06 h-mayorquin

Let's ping Sam for tomorrow and if he doesn't respond then I can fix the merge conflict. You want to give this one more careful read to make sure you understand the slicing logic?

zm711 avatar Jun 18 '25 21:06 zm711

I remember reading it back then. I can take a second look if you pint-point the specific part that you don't feel confident about.

h-mayorquin avatar Jun 18 '25 21:06 h-mayorquin

OK lets merge.

samuelgarcia avatar Jun 19 '25 06:06 samuelgarcia

Thanks @letiziasignorelli and sorry for the delay!

zm711 avatar Jun 19 '25 11:06 zm711