Uniform handling for stream/channel identifier changes across readers

[h-mayorquin]

This is a follow-up of #1604 ("Consistency of stream_index in neo.rawio") and comes from a discussion with @zm711 in #1874.

We need to provide consistency in the API for users to address our data-fetching methods (get_analogsignal_chunk, get_spike_timestamps, get_event_timestamps, rescale_signal_raw_to_float, and the future timestamps API discussed in #1773). Currently the API for most methods exposes addressing by index (stream_index, channel_indexes, spike_channel_index, event_channel_index) and, for signals, by id/name (channel_ids, channel_names). Both have changed before (Neuralynx #1786 / #1679, OpenEphysBinary #1624 / #1668 / #1753, SpikeGLX #1683 / #1661, Intan #1854 / #1558, Axon #1868 / #1874, EDF #1604, NeuroNexus #1711, Maxwell #1579, Plexon #1547) and this probably broke some user workflows. This is an ongoing effort, and while we try to provide backward compatibility (see the acrobatics I did on SpikeGLX to append the sync streams last and keep the stream order stable), it is on a best-effort basis and I am unsure how good a job we have been doing.

I would like to improve this and offer a contract that users can rely on. My proposal is that we do that only by id and not by index. The reason is that supporting index would 1) double the work and 2) force us to couple the API to our internal structures (an index is usually an internal detail of how the data is represented). Within that scope I think we should introduce new methods that are both stable and addressed only by id:

• get_analogsignal_chunk(stream_index, channel_indexes, channel_names, channel_ids) -> a new id-only method, e.g. get_analogsignal_chunk_by_id(stream_id, channel_ids)
• get_spike_timestamps(spike_channel_index) -> get_spike_timestamps_by_id(spike_channel_id)
• get_event_timestamps(event_channel_index) -> get_event_timestamps_by_id(event_channel_id)
• the future timestamps API (#1773) -> defined id-only from the start

Two notes:

• I am omitting block_index, segment_index, i_start/i_stop, t_start/t_stop and similar arguments from the signatures above; they stay the same in the new methods. This is only about how we address streams and channels. (The _by_id names are placeholders, up for discussion.)
• block_index and segment_index stay as indices on purpose. segment_index has no stable id to map to, because the number of segments is user-selection-dependent (it depends on the gap tolerance in the proposed timing API, #1773), so index is the only meaningful handle. block_index is a different axis that does not suffer this identifier churn, and multi-block is rare, so backward compatibility there is essentially free.

This will give us stable methods while keeping the current index-based ones, which are called inside them.

All of that said, it is a fact that the id/string identifiers need to change from time to time. Two examples:

1. We decided to move from streams grouped operationally (same sampling rate, dtype, and number of samples) to semantically meaningful streams, e.g. separating channels that have different physical units (the EDF case, where an EEG channel in uV and a respiration channel in a different unit end up in the same stream today) or by purpose (separating the sync channel in SpikeGLX, #1683). So we need a way to change things, and this has been happening.

2. We discovered on SpikeGadgets (#1847) that our names for the Neuropixels channels were wrong because they had been derived from the Intan case, so we needed to change them to be faithful to the acquisition (I described the exact id/name change here).

So on top of the new contract I also propose that we add machinery that works as a backward-compatibility layer: a stream and channel legacy map that captures old string identifiers, maps them to the new ones, and throws a warning. When this is not possible, e.g. the retired stream's channels are now split across streams with different sampling rates (so no single coherent chunk exists) or a channel was removed entirely, it should throw an informative error instead.

As a concrete example, I think the current EDF streams should be more specialized to be semantically meaningful. EDF currently groups channels by sampling rate, so one stream can mix an EEG channel in uV with a channel in a different unit. The proposal is to group by physical unit so each stream is homogeneous, and attach a legacy map from the old sampling-rate-based stream_id to the new unit-based streams, so old lookups still resolve and emit a warning pointing to the new streams.

Illustrative snippets (abstract, not final API)

EDF, group by physical unit and map the retired stream id:

# today: one stream per sampling rate, so units get mixed
streams = [("256Hz", "0")]            # stream "0" = EEG (uV) + SpO2 (%) together

# proposed: one stream per physical unit, plus a legacy map for the old id
streams = [("EEG", "eeg"), ("SpO2", "spo2")]
self._legacy_stream_mapper = LegacyStreamMapper({
    "0": {"channel_ids": [...], "since": "0.15",
          "reason": "streams now grouped by physical unit"},
})
# a lookup of the retired stream "0" still resolves, with a DeprecationWarning.

Axon, keep the real channel name and map the retired one:

# today: padding-stripping mangles multi-word names
name = raw.replace(b" ", b"")         # "IN 1" -> "IN1"

# proposed: keep the real name, map the retired one
name = raw.strip()                    # "IN 1"
self._legacy_channel_mapper = LegacyChannelMapper({
    "IN1": {"channel_id": "IN 1", "since": "0.15",
            "reason": "names now keep interior spaces"},
})
# addressing by the old "IN1" still resolves (warning); a removed channel raises.

[zm711]

I like this in general, but let's see what @alejoe91 and @apdavison say since they weigh in the most on these things. We will badger @samuelgarcia to look at this too.

[h-mayorquin]

Let's discuss this in a meeting. I think that's the right place for it.