Skip to content

Conversation

@alejoe91
Copy link
Member

@alejoe91 alejoe91 commented Dec 2, 2025

Something that's been on my todo list for a while.

This PR enables one to do recording1 +/- recording2 :)

@alejoe91 alejoe91 requested a review from h-mayorquin December 2, 2025 14:27
@alejoe91 alejoe91 added the core Changes to core module label Dec 2, 2025
Copy link
Collaborator

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

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

I think this will make the data model more expressive. Some edge cases:

  • Different number of segments.
  • Different number of samples
  • Different time parameters (either t_starts or explicit timestamps).

@alejoe91
Copy link
Member Author

alejoe91 commented Dec 3, 2025

I think this will make the data model more expressive. Some edge cases:

  • Different number of segments.
  • Different number of samples
  • Different time parameters (either t_starts or explicit timestamps).

First 2 are enforced and will raise an error if different. Time params are not, but maybe we should?

@h-mayorquin
Copy link
Collaborator

Yes, I think we should.

@samuelgarcia
Copy link
Member

Cool.
Maybe this could implemented in one class to avoid the copy/paste.

@h-mayorquin
Copy link
Collaborator

I think this will make the data model more expressive. Some edge cases:

  • Different number of segments.
  • Different number of samples
  • Different time parameters (either t_starts or explicit timestamps).

First 2 are enforced and will raise an error if different. Time params are not, but maybe we should?

Yes, in my first read I did not realize that this is done do_recording_attributes_match. I think that it can be a separated PR and discussion whether to add time basis to do the do_recording_attributes_match or add another function that checks that.

I am Ok with this.

@samuelgarcia samuelgarcia added this to the 0.104.0 milestone Dec 4, 2025
@alejoe91 alejoe91 merged commit 123e862 into SpikeInterface:main Dec 5, 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.

3 participants