qupath icon indicating copy to clipboard operation
qupath copied to clipboard

Feature request: Add subtract method to RoiTools

Open yau-lim opened this issue 2 years ago • 3 comments

Currently, RoiTools supports union and intersection methods directly but not subtraction.

Subtraction has to be done using combineROIs and CombineOp methods, while not difficult, is incoherent as union and intersection is directly available as mentioned above.

I would like to suggest having a RoiTools.subtract(roi1, roi2) method added alongside RoiTools.union(rois) and RoiTools.intersection(rois).

yau-lim avatar Jun 27 '22 12:06 yau-lim

Hi @yau-lim as discussed, I agree this should be added - it would be an easy addition to RoiTools, and requiring CombineOp is awkward (not least because it's not obvious that it exists...).

I suppose the difference with union and intersection is that they make sense with any number of ROIs (and so take a collection/list as input), but subtraction only really makes sense with two.... or at least that could have been what I was thinking originally.

With that in mind, one option is to add

public static ROI subtract(ROI roi1, ROI roi2) {
    return combineROIs(roi1, roi2, CombineOp.SUBTRACT);
}

as a convenience method. But I wonder if it might be even more useful to add

public static ROI subtract(ROI baseROI, ROI... roisToSubtract) {
  // Loop through one or more roisToSubtract, and remove then from baseROI
}

What do you think? Would this output match your expectation for 'subtraction that supports multiple arguments'?

The 'optional array' syntax proposed above would support all the following:

def roiOutput1 = RoiTools.subtract(roi1) // Returns roi1 unchanged
def roiOutput2 = RoiTools.subtract(roi1, roi2) // Returns roi1 with pixels in roi2 removed
def roiOutput3 = RoiTools.subtract(roi1, roi2, roi3, roi4) // Returns roi1 with pixels in the union of roi2, roi3 and roi4 removed

Alternatively, we could stick with just two arguments and then require

def roiOutput3 = RoiTools.subtract(roi1, RoiTools.union(roi2, roi3, roi4))

to get the third output above (albeit perhaps less efficiently).

petebankhead avatar Jun 27 '22 14:06 petebankhead

def roiOutput3 = RoiTools.subtract(roi1, RoiTools.union(roi2, roi3, roi4))

makes sense to me by building/layering functions together logically but from your other suggestions, maybe

public static ROI subtract(ROI baseROI, Collection<ROI> roisToSubtract) {
  // Loop through one or more roisToSubtract, and remove them from baseROI
}

will make it clear/less ambiguous to which ROIs are being subtracted from the baseROI, e.g.

def roiOutput = RoiTools.subtract(baseROI, [roi1, roi2, roi3, ...])

yau-lim avatar Jun 27 '22 15:06 yau-lim

Either way sounds good, or both ways - there have been a couple times when I expected this function to exist, was confused, and went and looked up that it did not :) And then forgot again, of course.

MichaelSNelson avatar Jun 27 '22 17:06 MichaelSNelson