Add neuronexus allego recording Extractor #3235
Conversation
zm711
added
extractors
|
|
||
| def __init__( | ||
| self, | ||
| file_path, |
There was a problem hiding this comment.
type hints while Sam is away. haha.
There was a problem hiding this comment.
Oh, I thought that he was against doing them himself I was not aware he dislikes them in principle : O
|
Reminder to myself: we changed it to NeuroNexus at Neo level so I need to fix this for the new name. |
|
C'est parfait. |
|
Fixed with a rerun. @h-mayorquin import windows failed with 3.05 seconds which was above the limit of 3 second set. Fixed on a re-run. I think we will have to watch out as you mess with numba and cuda--> those lead to really slow imports if not done carefully. Heberto and I will have to go back and fix things later :P |
|
|
||
| Parameters | ||
| ---------- | ||
| file_path : str |
There was a problem hiding this comment.
This can take path as well, right?
There was a problem hiding this comment.
Yes because we cast it to a path to resolve. I can add that :)
| If there are several streams, specify the stream name you want to load. | ||
| all_annotations : bool, default: False | ||
| Load exhaustively all annotations from neo. | ||
| use_names_as_ids : bool, default: False |
There was a problem hiding this comment.
Can we provide an illustrative examples of the names or are they too variable so giving a pattern does not make sense?
There was a problem hiding this comment.
I think we don't know the pattern super well. We only have the one test file at Neo, so I could write something, but I'm not confident.
There was a problem hiding this comment.
Let me know if you think it is still worth putting and I can combine your two pieces of feedback.
There was a problem hiding this comment.
Na, let's do it when we know more. No point in adding possible confusing message.
| @@ -52,8 +52,11 @@ def test_open(self): | |||
| num_samples = rec.get_num_samples(segment_index=segment_index) | |||
|
|
|||
| full_traces = rec.get_traces(segment_index=segment_index) | |||
| assert full_traces.shape == (num_samples, num_chans) | |||
| assert full_traces.dtype == dtype | |||
| assert full_traces.shape == ( | |||
There was a problem hiding this comment.
Something I like to do is to write expected_shape = (num_samples, num_channels) and then do the assertion so if I Jump in the debugger I have all the variables already loaded. But I think this is OK.
There was a problem hiding this comment.
Yeah I didn't change anything here. I was just getting an error and couldn't see what it was so I rage added this :)
There was a problem hiding this comment.
Yes, I realize, just sharing a pattern. I was going to suggest adding string to the message but this is the testing suite so I think this is fine and your addition is an improvement.
Co-authored-by: Zach McKenzie <[email protected]>
This depends on:
NeuralEnsemble/python-neo#1509
I still need to upload the test data to GIN and cleanup the PR, but I figure we can have for once that is merged :)