Skip to content

[BUG] crop_with_convex_hull fails when margin_thickness == 0.0 #99

@arnavsharma990

Description

@arnavsharma990

The Issue Description is Written with the help of Claude

Summary

When margin_thickness is set to 0.0, crop_with_convex_hull assigns the full tuple returned by create_convex_hull_mask(...) to da_mask, which later causes a type mismatch and runtime failure.

Location

mllam_data_prep/ops/cropping.py
Function: crop_with_convex_hull

Problem

In the zero-margin branch:

da_mask = create_convex_hull_mask(...)

However, create_convex_hull_mask returns:

(mask, hull_points)

Later in the same function:

_mask_with_common_dim(da_mask=da_mask, ds=ds)

_mask_with_common_dim expects da_mask to be an xarray.DataArray, but it receives a tuple instead.

This leads to a runtime error when the zero-margin path is triggered.

Why This Matters

Users may legitimately configure margin_thickness = 0.0 when they want cropping strictly along the convex hull boundary.

In this configuration the pipeline crashes, making this a correctness issue rather than just a minor edge case.

Suggested Fix

Unpack the return value explicitly in the zero-margin branch:

da_mask, _ = create_convex_hull_mask(...)

This keeps the expected DataArray type flowing into _mask_with_common_dim.

Possible Follow-up

It may also be helpful to add a regression test that runs the convex hull cropping with:

margin_thickness = 0.0

to ensure the code path remains stable.

Expected Impact

Small patch with high correctness impact. This change should not affect existing behavior except fixing the failure when zero margin is used.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions