Add NeuronexusRawIO/IO #1509
Add NeuronexusRawIO/IO #1509
Conversation
|
No rush on this @apdavison, this is a new io. Since Sam will be going on vacation, I think getting some feedback would be nice, but I can also pester him once he is back. There is no rush on this one :) |
| ####################################### | ||
| # Helper Functions | ||
|
|
||
| def read_metadata(self, fname_metadata): |
There was a problem hiding this comment.
I like to add path (hungarian notation) to differentiate between file handles (output of f = open() and paths) but I think name works as well!
There was a problem hiding this comment.
I think the parsing is super fast, but one thing I was thinking of was we could allow think about parsing more of header in this function then in spikeinterface or in neuroconv stuff you could run this function separately if you needed separate metadata then we could add a check in the main body that if this is already run we don't need to re-run. But the thought was inchoate (because honestly this function is so small right now). Maybe next time we have a call we can brainstorm on this because I don't think I'm explaining myself super well.
As far as naming, I just didn't want to use the same name so I added the fname tag onto it. But happy to change if it helps in conceptualizing more.
There was a problem hiding this comment.
That might be a good idea, I wish I knew more of the metadata to help you decide there.
Concerning the naming, it was just a random comment about what I do. I was saying that adding name makes sense as well!
|
To discuss here: expose logical streams instead of buffer streams for this format. |
|
It would be relatively easy. My only concern is that if we have a universal way to do this then we are adding more work to adapt all of these rawios into the new format. But in this case the last six channels are reserved for the non-ephys data. So I could split these off intro logical streams. |
|
Or we could merge this as is and then iterate for the logical streams in a new PR. That way if the buffer -> stream api happens before we have time then we fold that in and if this becomes an issue for users we can quickly fix it upon request before the api is added. |
|
@samuelgarcia, this one can also take a review in your queue. Just pinging you now that you're back here. We have a couple requests on spikeinterface (and on neuroconv) for this. At your leisure of course. |
zm711
requested review from
samuelgarcia and
alejoe91
and removed request for
apdavison
|
C'est le ping à Sam de la semaine. |
|
ceci n'est pas une ping |
|
He's on the thread. He'll see it :P |
| self.header["event_channels"] = event_channels | ||
|
|
||
| # Add the minimum annotations | ||
| self._generate_minimal_annotations() |
There was a problem hiding this comment.
Any chance to have a rec_datetime in the metadata dict ?
There was a problem hiding this comment.
I don't remember. Let me check!
There was a problem hiding this comment.
Okay so I'm adding to the header like this:
self.header['rec_datetime'] = self.metadata['status']['start_time']
It is setup like: date and time. Do you want any other info?
|
Salut Zach. |
Co-authored-by: Garcia Samuel <sam.garcia.die@gmail.com>
|
Thanks Sam. I think this is ready to go now after your feedback! |
neo/rawio/neuronexusrawio.py
Outdated
| # Add the minimum annotations | ||
| self._generate_minimal_annotations() | ||
|
|
||
| self.header['rec_datetime'] = self.metadata['status']['start_time'] |
There was a problem hiding this comment.
@zm711 could check this instead ?
the rec_datetime must be a real datatime and this must routed inside raw_annotaion at block and segment level for instance like this
| self.header['rec_datetime'] = self.metadata['status']['start_time'] | |
| rec_datetime = datetime.datetime(self.metadata['status']['start_time']) | |
| bl_annotations = self.raw_annotations["blocks"][0] | |
| seg_annotations = bl_annotations["segments"][0] | |
| for d in (bl_annotations, seg_annotations): | |
| d["rec_datetime"] = rec_datetime |
neo/rawio/neuronexusrawio.py
Outdated
| # date comes out as: | ||
| # year-month-daydayofweektime all as a string so we need to prep it for | ||
| # entering into datetime | ||
| # example: '2024-07-01T13:04:49.4972245-04:00' | ||
| stringified_date_list = self.metadata['status']['start_time'].split('-') | ||
| year = int(stringified_date_list[0]) | ||
| month = int(stringified_date_list[1]) | ||
| day = int(stringified_date_list[2][:2]) # day should be first two digits of the third item in list | ||
| time_info = stringified_date_list[2].split(':') | ||
| hour = int(time_info[0][-2:]) | ||
| minute = int(time_info[1]) | ||
| second = int(time_info[2]) | ||
| microsecond = 1000 * 1000 * (float(time_info[2]) - second) # second -> micro is 1000 * 1000 | ||
|
|
||
| rec_datetime = datetime.datetime(year, month, day, hour, minute, second, microsecond) | ||
| bl_annotations = self.raw_annotations["blocks"][0] | ||
| seg_annotations = bl_annotations["segments"][0] | ||
| for d in (bl_annotations, seg_annotations): | ||
| d["rec_datetime"] = rec_datetime | ||
|
|
There was a problem hiding this comment.
@samuelgarcia so I don't do as much date time stuff you want to look at this. Basically we get this string. I don't know what "T" inside the string or the 4:00 at the end (maybe duration in seconds?) so I did some ugly parsing. If you have a better idea from the parsing let me know!
There was a problem hiding this comment.
I guess my fear with this type of parsing is that if they change how they encode it we end up with giant parsing logic.
There was a problem hiding this comment.
2024-07-01T13:04:49.4972245-04:00
date_time_{T}_{timestamp}_{time_zone}
There was a problem hiding this comment.
@h-mayorquin if you have a good system could you actually make a PR off of this one and I'll merge it into this. I'm trash at this parsing and it seems like you have a better idea of how to pull this out!
There was a problem hiding this comment.
We are ready to merge this @h-mayorquin unless you have a better idea for parsing. Do you want me to hold off until you take a look or good to merge and we can prettify this bit later?
|
merci |
Fixes #1296.
I'll try to work on this a bit in my free time. Will switch off draft when ready for review.
Also should help for neuroconv