SpikeGLX: Add one box reading #1719
Conversation
|
Just to address this do we want to think about this in another PR? I think having stable streams is a better idea (like what we did with Intan). Especially if we can find some stability across the different formats (obx etc). |
|
Yes, we should do in another PR. |
| and local_chan == info["num_chan"] - 1 | ||
| ): | ||
| # Separate sync channel in its own stream | ||
| is_sync_channel = "SY" in chan_name and not self.load_sync_channel |
There was a problem hiding this comment.
This is for backward compatibility (searching for 'SY'?)
There was a problem hiding this comment.
I know better the metadata now and this is an easier criteria to determine if a channel is a synch channel.
| stream_id_for_chan, | ||
| buffer_id, | ||
| ) | ||
| ) | ||
|
|
||
| # all channel by default unless load_sync_channel=False | ||
| # None here means that the all the channels in the buffer will be included |
There was a problem hiding this comment.
should this comment indicate that this means the ephys + sync since the buffer in this case has the extra channel?
Not required, but I'm just imagining a future dev trying to figure this out.
There was a problem hiding this comment.
I am trying to document what assigning None to self._stream_buffer_slice[stream_id] means here because I think the statement:
self._stream_buffer_slice[stream_id] = Noneis very opaque.
I think once we deprecated load_sync_channel this section will be more clear but right now I am struggling to right all the conditions. Feel free to make suggestions.
| @@ -404,9 +399,21 @@ def scan_files(dirname): | |||
| if len(info_list) == 0: | |||
| raise FileNotFoundError(f"No appropriate combination of .meta and .bin files were detected in {dirname}") | |||
There was a problem hiding this comment.
We should improve this eventually. If I'm a user of neo and I see one bin and one meta file in my folder I would assume it should work. So the word "appropriate" is not actionable in this case. Not for this PR but I don't want us to forget this.
There was a problem hiding this comment.
Yeah, a better error here would be good.
neo/rawio/spikeglxrawio.py
Outdated
| """ | ||
| re_standard = re.findall(r"(\S*)_g(\d*)_t(\d*)\.(\S*).(ap|lf)", fname) | ||
| re_tcat = re.findall(r"(\S*)_g(\d*)_tcat.(\S*).(ap|lf)", fname) | ||
| re_nidq = re.findall(r"(\S*)_g(\d*)_t(\d*)\.(\S*)", fname) | ||
| re_obx = re.findall(r"(\S*)_g(\d*)_t(\d*)\.(\S*)\.obx", fname) |
There was a problem hiding this comment.
can a note be added for these regexes? I am never able to read them and so I have to look up each rule each time when I troubleshoot something.
There was a problem hiding this comment.
Let me try to improve them. I agree with you that most people are not innate regex readers. I am certainly not.
There was a problem hiding this comment.
I improved the whole function. But maybe this commit should be a PR on it own. Let me know what you think.
There was a problem hiding this comment.
For me that's super useful. It is not necessarily within the scope of the PR, but I say we keep it unless @samuelgarcia or @alejoe91 really want it in another PR.
|
If you want to wait for the others to read it. Let's remove it, and I will make another PR after to make this easier to read |
Should close #1684
Needs the gin test data that is in a PR right now:
https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/pulls/156#