NanoComp / imageruler

measure minimum solid/void lengthscales in binary image
https://nanocomp.github.io/imageruler/
MIT License
13 stars 6 forks source link

Minimal API for imageuler #25

Closed mfschubert closed 7 months ago

mfschubert commented 7 months ago

In the context of fixes and potential improvements to the imageruler (#24 , #22 , #23 ), it may be worth reconsidering the API. Currently, the API is a bit complex, accommodating e.g. periodic boundary conditions, optional border exclusion for violation detection, padding modes, kernel shapes, and non-square pixels in the top-level functions.

def minimum_length_solid_void(
    array: Array,
    phys_size: Optional[PhysicalSize] = None,
    periodic_axes: Optional[PeriodicAxes] = None,
    margin_size: Optional[MarginSize] = None,
    pad_mode: Tuple[PaddingMode, PaddingMode] = (PaddingMode.SOLID, PaddingMode.VOID),
    kernel_shape: KernelShape = KernelShape.CIRCLE,
    warn_cusp: bool = False,
) -> Tuple[float, float]:
    ...

Some of the previously-identified issues only arise when these features are exercised. In the photonics-opt-testbed (code search), only periodic boundaries are used, and so it seems we could get away with something like,

def minimum_length_solid_void(
    array: Array,
    periodic_axes: Optional[PeriodicAxes] = None,
) -> Tuple[float, float]:
    ...

Aiming for a simpler API should make it easier to develop a well-tested and reliable implementation of the imageruler algorithm. I would be happy to contribute improvements to the codebase, but believe it is probably worth discussing larger changes before submitting a PR. In particular, it would be good to hear if there is a case for retaining the the phys_size, margin_size, pad_mode, and kernel_shape arguments.

stevengj commented 7 months ago

This makes a lot of sense to me.

At least some of the features can be changed from function parameters to function compositions — e.g. padding can be a separate function, resizing to square pixels can be a separate function.

And some parameters like kernel_shape they only seem to have been needed in the research phase.

stevengj commented 7 months ago

Closed by #30