Use names as ids in Plexon1 recording extractor #3193
Conversation
| The file path to load the recordings from. | ||
| stream_id : str, default: None | ||
| If there are several streams, specify the stream id you want to load. | ||
| stream_name : str, default: None | ||
| 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: True | ||
| If True, the names of the signals are used as channel ids. If False, the channels ids are the ids provided |
There was a problem hiding this comment.
Is this name of the signal or channel_names? In neo we would call them channel_names, but do you thing signals is more parseable for our spikeinterface users?
There was a problem hiding this comment.
channel_names check the name of the field in the structure array for signals that I shared below and the code:
There was a problem hiding this comment.
I mean in the docstring is the word signals the best word or do we say the Neo term and say use the channel_names as provided by Neo. Based on the neo issue we've got linked I don't think semantically our name vs id vs index is super clear similar to how the stream semantic is a bit hard to follow. So what is the best way for this type of docstring to clearly convey what a channel_name means so people can honestly make a choice.
For example for intan the difference between name and id is actually the difference between using the custom user name and the hardware native name. Here for plexon it is the difference between an actual hardware channel name and a stringified index. So there isn't really a unified meaning to use_names_as_ids to be clear for our end-users.
There was a problem hiding this comment.
But to be clear this shouldn't slow down this PR, but I think we need to come up with a better way to explain this.
There was a problem hiding this comment.
No, no, I think you are right. Sorry I misread. Yes, the docstring should be changed. Given that there is no unified semantics we need something better. Let me make a commit and see what you think. It is good to discuss that because I am enabling this attribute for all extractors (not changing the default behavior like here though) so users don't need to wait for us if they want to choose.
Give me a sec.
There was a problem hiding this comment.
Check the new one. in the latest commit. It does not make any promises and is quite literal. Then, when we know (as it takes some work and knowledge of the formats) we can add clarifications about the semantics and example differences as we do here.
So the template of the docstring is:
- The abstract "I don't make any claims" think here.
- Some clarification about what each of them are (for example in Intan we can mentioned what is what).
- An example of how they look like.
What do you think?
There was a problem hiding this comment.
Yeah, I think that is best. It's a lot of work for all neo.rawios, but I guess we are little stuck by the nature of the neo names vs ids. Thanks!
There was a problem hiding this comment.
Yeah, I am gonga propagate everything with 1. And hopefully we can address 2 and 3 slowly but surely.
Gradatim Ferociter
|
Oh no windows imports again! Hope is a fluke : ) This is related to: And this are the channel ids and names that @AbhiSwamiUConn shared with us , 'signal_channels': array([('WB01', '0', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB02', '1', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB03', '2', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB04', '3', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB05', '4', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB06', '5', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB07', '6', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB08', '7', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB09', '8', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB10', '9', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB11', '10', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB12', '11', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB13', '12', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB14', '13', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB15', '14', 40000., 'int16', '', 0.00019488, 0., '1'),
('WB16', '15', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC01', '16', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC02', '17', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC03', '18', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC04', '19', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC05', '20', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC06', '21', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC07', '22', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC08', '23', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC09', '24', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC10', '25', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC11', '26', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC12', '27', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC13', '28', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC14', '29', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC15', '30', 40000., 'int16', '', 0.00019488, 0., '1'),
('SPKC16', '31', 40000., 'int16', '', 0.00019488, 0., '1'),
('FP01', '32', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP02', '33', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP03', '34', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP04', '35', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP05', '36', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP06', '37', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP07', '38', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP08', '39', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP09', '40', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP10', '41', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP11', '42', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP12', '43', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP13', '44', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP14', '45', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP15', '46', 10000., 'int16', '', 0.00019488, 0., '0'),
('FP16', '47', 10000., 'int16', '', 0.00019488, 0., '0')],
dtype=[('name', '<U64'), ('id', '<U64'), ('sampling_rate', '<f8'), ('dtype', '<U16'), ('units', '<U64'), ('gain', '<f8'), ('offset', '<f8'), ('stream_id', '<U64')]),
``` |
Co-authored-by: Zach McKenzie <[email protected]>
Related to this discussion in neo:
NeuralEnsemble/python-neo#1507