imglib / imglib2-roi

Regions of interest (ROIs) and labelings for ImgLib2
Other
8 stars 8 forks source link

Ambiguous sample method #57

Closed axtimwalde closed 4 years ago

axtimwalde commented 4 years ago

This code used to work as expected:

final WritablePolygon2D polygon = GeomMasks.closedPolygon2D(x, y);
for (final UnsignedLongType t : Regions.sample(polygon, target)) ...

Now I get a

The method sample(RealMaskRealInterval, RandomAccessible<UnsignedLongType>) is ambiguous for the type Regions

because you now have now two methods

Regions.sample(RealMask, RandomAccessibleInterval)
Regions.sample(RealMaskRealInterval, RandomAccessible)

which is very unfortunate because RandomAccessibleInterval extends RandomAccessible and RealMaskRealInterval extends RealMask, which means the compiler can never find out what to do without super verbose casting, and in scripts with dynamic type inference you will be screwed.

It can also lead to unexpected inefficiencies, because you may end up using

sample(RealMask, RandomAccessibleInterval)

which loops over the entire target interval instead of

sample(RealMaskRealInterval, RandomAccessible)

which loops only over the bounding box of the region. All of that without notice.

I suggest to remove the ambiguous new method

sample(RealMask, RandomAccessibleInterval)

and replace it by

sampleInterval(RealMask, RandomAccessibleInterval)

unfortunately, you will have to break this hard, because deprecation would keep this issue alive.

An alternative would be to deprecate both sample methods and replace them by

sampleInterval(RealMask, RandomAccessibleInterval)
sampleRegion(RealMaskRealInterval, RandomAccessible)

but I can see how calling them sample was attractive...

axtimwalde commented 4 years ago

Turns out this is fixed in https://github.com/imglib/imglib2-roi/commit/d6eb24cddd6517436f2115be6c748b59ff11be46

axtimwalde commented 4 years ago

Permaturely closed, this was a related issue that tricked @tpietzsch and me to briefly believe that the issue was already addressed.

axtimwalde commented 4 years ago

Fixed by https://github.com/imglib/imglib2-roi/commit/a0492097670e2d6902e53d1ede3c82ef0655aab8

Thanks!