Skip to content

Add the ProbeGroup._global_contact_order concept.#446

Open
samuelgarcia wants to merge 8 commits into
SpikeInterface:mainfrom
samuelgarcia:group_order
Open

Add the ProbeGroup._global_contact_order concept.#446
samuelgarcia wants to merge 8 commits into
SpikeInterface:mainfrom
samuelgarcia:group_order

Conversation

@samuelgarcia

@samuelgarcia samuelgarcia commented Jun 9, 2026

Copy link
Copy Markdown
Member

#416 added a get_slices() at ProbeGroup level but this PR has enhanced after long debates and discussions the fact that the ProbeGroup has a strong problem : the order of contact with to_numpy()/from_numpy() or to_dict()/form_dict() can be lost!

This can append when contact of sevral Probe are interleaved. This was not possible before but given the Probegrorup.get_slice() contacts of several can be interleaved!

This PR add the hidden concept of ProbeGroup._global_contact_order.

  • this is backward compatible because None by default (aka natural contact order)
  • make possible pg2 = ProbeGroup.from_numpy(pg1.to_numpy(complete=True)) to maintain any abritory order
  • make possible pg2 = ProbeGroup.from_dict(pg1.to_dict()) to maintain any abritory order

The get_slice is now on top of to_numpy() after the ordering.

This PR is validated:

Important note: the _global_contact_order is NOT the global_device_channel_indices. It can be any order.
(Even if the main use case is to order by global_device_channel_indices on spikeinterface side).

In short, this PR garanty the order after the json write/read.

@alejoe91 @chrishalcrow @h-mayorquin

Test need to be done after validation.

@alejoe91

alejoe91 commented Jun 9, 2026

Copy link
Copy Markdown
Member

Thanks camarade. Can you fix tests?

@alejoe91

alejoe91 commented Jun 9, 2026

Copy link
Copy Markdown
Member

Comment thread src/probeinterface/probegroup.py Outdated
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
@h-mayorquin

h-mayorquin commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

A clarification. I don't think my PR #425 here in probeinterface is superseded by this. Not that I am attached to keeping it, but to be clear: that PR is about providing a small dense representation so people can index on it in an array-like way to fetch properties. That is the full goal of it. In a nutshell, the full numpy representation is too much; let's provide a small surface area, commensurate with what SpikeInterface needs, that achieves that goal.

Now regarding this PR. The idea of having a way of preserving the order through to_numpy/from_numpy and to_dict/from_dict, and giving the JSON file a defined display order, seems reasonable to me. More importantly, _global_contact_order seems like a fine internal lever for it. I read it as an implementation detail we can set and apply internally in whatever scenario needs it, without changing the public surface, and I think this is a good idea because it frees device_channel_indices from this burden.

Some small comments:

  • global_contact_order.to_list() should be .tolist() (it is annoying that pandas and numpy are different in this regard.)
  • the docstring of set_global_device_channel_indices still refers to the old channels parameter after the rename to device_channel_indices.
  • The annotation metadata should be fixed first; let's not leave this as a TODO.

One big problem that I would like to point out:
I think get_slice suffers from the same array-based confusion that I have been arguing against. The reason the refactor here is touching it so much is because it is working under the same implicit semantics of it being an array: its arguments are positional integers. To me, this is a bad design pattern that keeps recurring. To provide our users with a full object with all the metadata, our libraries merge data from different sources: the recording, the probe, and possibly others in the future such as headstage information. The standard design in databases is to index by a key, not by the positional integer of the channel in the source. Indexing by an integer position in the representation of the object couples the user interface to the internal representation of the data, which should not be exposed directly to users, as it leads to errors (for example SpikeInterface #4565, #4545, #4546, #4547).

To avoid the pattern above, get_slice at the Probe level should work with contact_ids, and the ProbeGroup level can either rely on that or have get_slice take (probe_id, contact_id) pairs. This would solve the metadata problem above, because the data can then be sliced faithfully and unambiguously, and it would stop pretending that the ProbeGroup is an array. To be clear, my objection is specifically that the public API should remove its array-like components; keeping the order internally, as _global_contact_order does, is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants