losonczylab / sima

Python package for analysis of dynamic fluorescence microscopy data
GNU General Public License v2.0
100 stars 50 forks source link

Extracting signals from weighted ROIs fails #140

Open neurodroid opened 9 years ago

neurodroid commented 9 years ago

Weighted ROIs, as returned by sima.segment.STICA, cannot be used for signal extraction. Using r178576528. Minimal standalone sample to reproduce the issue:

import sima
import sima.segment
import sima.misc
from shutil import copy, copytree

copytree(sima.misc.example_data(), 'example.sima')
copy(sima.misc.example_tiff(), 'example.tif')

dataset = sima.ImagingDataset.load('example.sima')

# The following works, indicating that the ROIs are not "empty":
stica_sparse_approach = sima.segment.STICA(components=19, verbose=True)
stica_sparse_approach.append(sima.segment.SparseROIsFromMasks())
stica_sparse_rois = dataset.segment(stica_sparse_approach, 'stica_sparse_ROIs')
signals = dataset.extract(stica_sparse_rois, signal_channel='green', 
    label='green_signal_stica_sparse')

# The following fails
stica_approach = sima.segment.STICA(components=19, verbose=True)
stica_rois = dataset.segment(stica_approach, 'stica_ROIs')
signals = dataset.extract(stica_rois, signal_channel='green', label='green_signal_stica')

Relevant messages:

UserWarning: Empty ROIs will return all NaN values: 19 empty ROIs found
ValueError: No valid ROIs found.
pkaifosh commented 9 years ago

Thanks for this bug report of exemplary clarity.

The extract method has a remove_overlap argument that defaults to True, causing it to remove all overlapping pixels are common to more than one ROI. In the case of mask ROIs, which have values for all pixels, this removes everything.

The error goes away if you change the value of this argument to be False:

signals = dataset.extract(stica_rois, signal_channel='green', label='green_signal_stica', remove_overlap=False)

@jzaremba @nbdanielson Would you object to changing the default value for remove_overlap to be False?

jzaremba commented 9 years ago

I'm somewhat hesitant to change the default behavior, not removing overlapping pixels can actually have a fairly substantial effect on your signals if you expected them to have been removed.

That being said, I'm not sure you'd ever want to remove overlapping pixels for non-boolean masks. We could just disable that ability entirely and throw a warning if you try it?

neurodroid commented 9 years ago

That being said, I'm not sure you'd ever want to remove overlapping pixels for non-boolean masks. We could just disable that ability entirely and throw a warning if you try it?

+1