search

imagej / imagej-common

ImageJ core data model
https://imagej.net/libs/imagej-common
BSD 2-Clause "Simplified" License
9 stars 18 forks source link

Roitree to mask converters #101

Closed gselzer closed 2 years ago

gselzer commented 2 years ago

This PR adds a set of Converters enabling conversions between single-ROI ROITrees and RealMasks of different types.

gselzer commented 2 years ago

Thanks @imagejan for the suggestions! I liked them all, and fixed them up!

gselzer commented 2 years ago

@ctrueden is there anything holding this back from merge?

gselzer commented 2 years ago

I'm wondering why we need converters from ROITree to single RealMasks.

The main use case follows from this decision

ctrueden commented 2 years ago

After discussion in person: I really don't know a better way, so for the moment, we move forward.

ctrueden commented 2 years ago

I'm a little confused now. Wasn't the type -> type way of conversion deprecated in SJC2, precisely because we cannot always guarantee the conversion will work without knowing the object that needs to be converted? Or do I misunderstand your point here?

No, you're right on, @imagejan. I was/am just concerned because despite that deprecation, there are places in the system that still rely on those functions to make decisions. It turns out to be kind of challenging to update SciJava core across the board because there are cases where we don't have an existing object handy yet to ask which conversions are available.

@gselzer and I discussed two ways forward for the long term: A) get rid of the type → type conversion checking API (the path we already started on, as you point out); or B) get rid of the object instance → type contingent conversion support e.g. as used in this PR. We concluded that (B) is not really feasible without wrecking some of the current uses of the conversion framework, so let's continue toward (A).