Fix spikeglx stream for isolated folder from multi probe case. - #1661
Conversation
|
@guptadivyansh, so this was a loading regression that occurred. So we also want to fix this. Depending on which PR gets merged first you two might end up with some conflicts. So just want you to take a look at the tweak we are making here as well. |
| for info in info_list: | ||
| if info.get("device") == "nidq": | ||
| info["device_index"] = "" # TODO: Handle multi nidq case, maybe use meta["typeNiEnabled"] | ||
| if info.get("device_kind") == "imec": |
There was a problem hiding this comment.
why switch back to imec. Just so I can better understand this for the future.
There was a problem hiding this comment.
I just switch the order of the if here and the diff is not working as it should.
But overall, instead of calculating the stream_index by considering all the possible configurations of probe, dock, slot we are just parsing it from the file name that is written on the metadata.
There was a problem hiding this comment.
Okay I'm just showing off my total ignorance of this IO. I see how you flipped it now.
There was a problem hiding this comment.
maybe a comment saying the options are nidq or imec above the if-else? In case other devs work on this in the future??
There was a problem hiding this comment.
Feel free to suggest a line that would make it easier for you as you are in a better position to judge what's helpful.
I will revisit this once we are adding support for one box data:
https://www.neuropixels.org/_files/ugd/328966_9d776efdcff74658a3724690610d4b41.pdf
|
@grahamfindlay do you have a moment to test this PR really quick? Since we are just reverting due to lost behavior and we've added a test I feel like we are in a good place, but having an end-user test would be awesome. |
|
@zm711 it works for me! |
|
Great fixing the CI tomorrow-ish and then we can merge after we retest :) |
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
zm711
left a comment
zm711
left a comment
There was a problem hiding this comment.
Two separate users confirm this fixes the regression. Merging.
3 participants
Should fix #1661
See discussion here: #1656
Added a test as @zm711 requested.
This is restoring some behavior that we lost after #1608
Plus, reading the metadata history changes here:
https://billkarsh.github.io/SpikeGLX/help/parsing/
Calculating
device_indexthrough the original file name is robust up till the phase 3A whereas the currentprobe_indexcalculation might fail for some phases. So I think this is a good improvement overall and simplifies the code.