Set max_channel property of MdaSortingExtractor #3701
Conversation
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
|
I think the comment about max_channel values being 1-indexed and being based on the input data to the sorter is useful to document somewhere. Perhaps in the docstring of |
|
I haven't run tests locally. I was hoping to see if the workflows pass. |
| If sorting was run on a subset of channels of the recording, then the max_channel values are | ||
| based on that subset, so care must be taken when associating these values with a recording. | ||
| If additional sorting segments are added to this sorting extractor after initialization, | ||
| then max_channel will not be updated. The max_channel indices begin at 1. |
There was a problem hiding this comment.
Should we then set the property with a -1?
There was a problem hiding this comment.
Probably. Though if you are familiar with the MDA format, you might know and expect that the channel indices begin at 1. I leave that for you to decide
There was a problem hiding this comment.
No strong preference here! Just thinking that if downstream or during NWB export we assume these are channel indices we would assume they start from 0.
@magland any thought here?
There was a problem hiding this comment.
Letàs leave 1-based, since this is a reader :)
|
Is there a way to add a description to the |
|
@rly no unfortunately we don't have property descriptions. I think that this could be done with a custom snippet on the NeuroConv side |
Fix #3695