Skip to content

Handle empty sequences in convert_ras_hdf_value#101

Open
arpitjain099 wants to merge 1 commit into
fema-ffrd:mainfrom
arpitjain099:fix/convert-empty-sequence
Open

Handle empty sequences in convert_ras_hdf_value#101
arpitjain099 wants to merge 1 commit into
fema-ffrd:mainfrom
arpitjain099:fix/convert-empty-sequence

Conversation

@arpitjain099

Copy link
Copy Markdown

convert_ras_hdf_value() in src/rashdf/utils.py raises IndexError when it is
handed an empty sequence. The list/tuple/ndarray branch only distinguishes
len(value) > 1 from everything else, so an empty value falls into the
single-element else branch and indexes value[0]:

elif isinstance(value, (list, tuple, np.ndarray)):
    if len(value) > 1:
        return [convert_ras_hdf_value(v) for v in value]
    else:
        return convert_ras_hdf_value(value[0])  # value[0] -> IndexError when empty

An empty array is easy to hit when an HDF attribute or dataset carries no
elements, and the resulting IndexError is harder to trace back than a clean
return. This adds an explicit len(value) == 0 case that returns None, which
matches how the function already maps absent values (NaN becomes None) and
stays inside the declared return type.

Verification with pytest on the affected test:

  • Before the fix, tests/test_utils.py::test_convert_ras_hdf_value fails with
    IndexError: list index out of range at src/rashdf/utils.py:295.
  • After the fix it passes, and the full tests/test_utils.py module is green
    (6 passed).

The test adds coverage for the empty list, tuple, and numpy array inputs.

I hit some unrelated failures in test_geom.py and test_plan.py in my local
environment, but they reproduce on a clean checkout of main as well (they look
like geopandas/zarr version differences), so they are not related to this
change.

An empty list, tuple, or numpy array fell through the len(value) > 1 check into the single-element branch and indexed value[0], raising IndexError. Return None for the empty case, consistent with how the function already maps absent values such as NaN to None, and add a test covering empty list, tuple, and array inputs.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant