Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency in the purposes of MultiImagingExtractor and MultiSegmentationExtractor #178

Open
h-mayorquin opened this issue Jul 20, 2022 · 0 comments

Comments

@h-mayorquin
Copy link
Collaborator

h-mayorquin commented Jul 20, 2022

TLDR;
MultiImagingExtractor handles the concatenation of recordings of different imaging extractors from the same imaging plane.
MultiSegmentationExtractor handles a collection of segmentation extractors that correspond to different imaging planes (without concatenation).

The issue

So, I was reading this issue to understand the purpose of MultiSegmentationExtractor as it seems to be handled in a very different way in nwb conversion tools to anything else I have seen:

#11

Right now, the purpose of the MultiImagingExtractor that was added in #127 is to concatenate recordings in time. We have some images and the MultiImagingExtractor handles them as if they came from the same device and appends them over time.

However, the MultiSegmentationExtractor does something different, it is mean to handle segmentation in different planes in a common extractor object. For example, it does not concatenate the signals (raw traces , df/f, etc) in time it just adds more of them to the common get_traces_dict as further signals:

from roiextractors.testing import generate_dummy_segmentation_extractor
segmentation_extractor1 = generate_dummy_segmentation_extractor(num_columns=25, num_rows=25, num_rois=3)
segmentation_extractor2 = generate_dummy_segmentation_extractor(num_columns=25, num_rows=25, num_rois=5)

from roiextractors import MultiSegmentationExtractor

segmentatation_extractors_list = [segmentation_extractor1, segmentation_extractor2]
multi_extractor = MultiSegmentationExtractor(segmentatation_extractors_list=segmentatation_extractors_list)

multi_extractor.get_traces_dict().keys()

Output:

dict_keys(['raw_Plane0', 'dff_Plane0', 'neuropil_Plane0', 'deconvolved_Plane0', 'raw_Plane1', 'dff_Plane1', 'neuropil_Plane1', 'deconvolved_Plane1'])

It seems that the original intention on issue #11 was to implement MultiImagingExtractor in such way (to handle different imaging extractors corresponding to different planes in a single object).

What to do?

Maybe we can change the name of MultiSegmentationExtractor to MutliPlaneSegmentationExtractor and keep it as it is? As things stand, it seems confusing and inconsistent to keep analogous names for objects that have a different purpose. Another option would be to rename MultiImagingExtractor to something like ConcatenatedImagingExtractor.

spikeinterface has the concepts of segments to do what our current MultiImagingExtractor does if that should be used as guidance.

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

No branches or pull requests

1 participant