Skip to content

Refactor probegroup to be array-based#420

Open
alejoe91 wants to merge 2 commits intoSpikeInterface:mainfrom
alejoe91:probegroup_as_array
Open

Refactor probegroup to be array-based#420
alejoe91 wants to merge 2 commits intoSpikeInterface:mainfrom
alejoe91:probegroup_as_array

Conversation

@alejoe91
Copy link
Copy Markdown
Member

In #416 we extended the ProbeGroup with a get_slice method. However, the current probe-centric implementation makes it difficult to slice in device channel indices from different probes are interleaved.

This PR refactors the ProbeGroup to be based on a _self.contact_array:

  • when a new probe is added, its array presentation is added to the _self.contact_array and we store probe contours and annotations (contact_annotations are in the array)
  • the probes is not a property that reconstructs the probes on-the-fly
  • importantly, changing annotations and contact annotations for a probe in a probegroup is now disabled. Added an annotate_probe(probe_index, **kwargs) to annotate probes in the probe group

Related to SpikeInterface/spikeinterface#4465, since we want to store a probegroup object directly, but we need to sort it/silce it according to the device_channel_indices

from .probe import Probe


class ProbeGroup:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alejoe91 we should make a very clear doc here for the new behavior.
We could explain teh why (need of global conact order across probes)
And also metioning the previous behavior (list of probe) and what cannot be possible now (annotate after)

@samuelgarcia
Copy link
Copy Markdown
Member

There is still an issue with this PR we need to solve : the to_dict/from_dict() and json will NOT preserve the contact order... We need to find something for it!!!

Give me a chance to add more complex test for orderings the probe group and global channel indices.

I propose a switch of version to 0.4.

@h-mayorquin
Copy link
Copy Markdown
Collaborator

I am taking a look. I will publish a review later today

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