Skip to content

Conversation

@RobertoDF
Copy link
Contributor

@RobertoDF RobertoDF commented May 27, 2025

Fix #3947

@RobertoDF RobertoDF changed the title fix #3947 keep probe_planar_contour aggregating channels May 27, 2025
@RobertoDF RobertoDF changed the title keep probe_planar_contour aggregating channels keep probe_planar_contour when aggregating channels May 27, 2025
@chrishalcrow
Copy link
Member

Hello, looking great - good bug catch! There are other annotations we're not forwarding during this function (like probes_info). It'd be amazing if your code generalised to all annotations. I think your basic structure will abstract pretty nicely to all annotations. You'd do something like

for annotation_key in recording.get_annotation_keys():
    check annotation_key is a key in all sub-recordings
    check annotation_value is the same for all sub-recordings
    If it is:
        set the annotation (provided it's not already set: e.g is_filtered gets set somehow)

I think the hard thing is to make the all function work more generally. In my testing, something like

all( np.all(k == first_annotation_value) for k in annotation_values))

seems ok, but maybe someone has a better idea.

@samuelgarcia
Copy link
Member

Thanks for this.
I am ok with it.
ANd also ok with @chrishalcrow comments.

@RobertoDF
Copy link
Contributor Author

@chrishalcrow I think your idea will work well with small modifications from what I proposed. I won´t have time to test this this week though!

@alejoe91 alejoe91 added the core Changes to core module label Jun 3, 2025
@h-mayorquin
Copy link
Collaborator

I would like to point out that this is a data point of just passing the whole probe as an attribute and fetch the information from the probe when it is available. Spreading the probe information across annotations and properties makes this type of error harder to trace.

@alejoe91 alejoe91 added this to the 0.103.0 milestone Jun 11, 2025
@samuelgarcia
Copy link
Member

thanks you. Lets make an issue to make it for all annotations

@samuelgarcia samuelgarcia merged commit 91ca0d6 into SpikeInterface:main Jun 12, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes to core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

probe_planar_contour lost when splitting & aggregating channels

5 participants