Skip to content

feat(zettasets): Fall back to zettasets images#26

Open
nkemnitz wants to merge 1 commit intomtc_ddpfrom
nkem/fix-zettasets-src
Open

feat(zettasets): Fall back to zettasets images#26
nkemnitz wants to merge 1 commit intomtc_ddpfrom
nkem/fix-zettasets-src

Conversation

@nkemnitz
Copy link
Collaborator

No description provided.

@nkemnitz nkemnitz requested a review from torms3 February 20, 2026 11:40
Copy link

@torms3 torms3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control flow via exception (Claude Code)

Using raise ValueError just to catch it a few lines later is a code smell:

if sample.src_image_path is None:
    raise ValueError("No src_image_path available")
# ...
except (InfoUnavailableError, ValueError) as e:

This also has the side effect of catching genuine ValueErrors from CloudVolume internals. Consider restructuring to avoid exceptions as control flow:

vol = None
if sample.src_image_path is not None:
    try:
        vol = CloudVolume(
            sample.src_image_path,
            mip=resolution,
            fill_missing=True,
            bounded=False,
        )[image_bbox.to_slices()]
    except InfoUnavailableError as e:
        logging.warning(f"src_image_path failed ({e}), falling back to sample image volume")

if vol is None:
    vol = CloudVolume(
        sample.volumes["image"].cloudpath,
        mip=resolution,
        fill_missing=True,
        bounded=False,
    )[image_bbox.to_slices()]

This makes the None check and the InfoUnavailableError handling separate concerns, and avoids accidentally swallowing unrelated ValueErrors.

Unguarded fallback (@torms3)

The fallback assumes sample.volumes["image"] always exists. If it doesn't, the user gets an unhandled KeyError with no context. Consider validating before accessing:

if "image" not in sample.volumes:
    raise RuntimeError(
        f"src_image_path failed ({e}) and sample has no 'image' volume to fall back to"
    ) from e

This applies to both multi_zettaset.py and zettaset.py.

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.

2 participants