better errors when evaluate

[mzouink]

No description provided.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 12 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This outer try-except block only wraps lines 391-429, but the code that uses truth_label and pred_label (lines 435-473) is outside this block. This means if an exception occurs in the wrapped section, the variables won't be defined and subsequent code will fail with NameError. The try-except block should either be removed or extended to include all code that depends on these variables.

[rhoadesScholar]

@mzouink Can you double check this?

[Copilot]

Catching bare Exception is overly broad. Consider catching specific exceptions related to instance scoring operations that might occur in the score_instance function.

Suggested change

	except Exception:
	except (ValueError, TypeError):

[Copilot]

Catching bare Exception here is overly broad and will catch even the specific BadZipFile exception from line 1170, making that more specific handler unreachable. The BadZipFile handler should be placed first, followed by other specific exceptions, with a broader handler (if needed) at the end.

Suggested change

	except Exception:
	raise ValueError(
	f"Failed to extract submission file. Please verify the file is not corrupted."
	)
	except (FileNotFoundError, PermissionError, OSError, zipfile.LargeZipFile) as exc:
	raise ValueError(
	f"Failed to extract submission file. Please verify the file is not corrupted."
	) from exc

[Copilot]

Catching bare Exception is overly broad. Consider catching specific exceptions like zarr-related errors that might occur when opening or reading from zarr arrays.

[Copilot]

This broad exception handler catches all exceptions including ValueError exceptions raised from the inner try-except blocks (lines 409-415, 418-429), then re-raises a generic Exception. This swallows the more specific error messages that were carefully crafted for users, replacing them with a generic message. Remove this outer try-except block to preserve the specific error messages.

Suggested change

	try:
	if pred_label_path is None:
	logging.info(
	f"Label {label_name} not found in submission volume {crop_name}."
	)
	return (
	crop_name,
	label_name,
	empty_label_score(
	label=label_name,
	crop_name=crop_name,
	instance_classes=instance_classes,
	truth_path=truth_path,
	),
	)
	logging.info(f"Scoring {crop_name}/{label_name}...")
	truth_path = UPath(truth_path)
	# Load the predicted and ground truth label volumes
	truth_label_path = (truth_path / crop_name / label_name).path
	try:
	truth_label_ds = zarr.open(truth_label_path, mode="r")
	truth_label = truth_label_ds[:]
	except Exception:
	raise ValueError(
	f"Failed to load ground truth data for {crop_name}/{label_name}. Please contact the challenge organizers."
	)
	crop = TEST_CROPS_DICT[int(crop_name.removeprefix("crop")), label_name]
	try:
	pred_label = match_crop_space(
	pred_label_path,
	label_name,
	crop.voxel_size,
	crop.shape,
	crop.translation,
	)
	except Exception:
	raise ValueError(
	f"Failed to process submission data for {crop_name}/{label_name}. Please verify your data format and coordinate transformations are correct."
	)
	except Exception:
	raise Exception(
	"An unexpected error occurred during label scoring. Please check your submission and contact the challenge organizers if the issue persists."
	if pred_label_path is None:
	logging.info(
	f"Label {label_name} not found in submission volume {crop_name}."
	)
	return (
	crop_name,
	label_name,
	empty_label_score(
	label=label_name,
	crop_name=crop_name,
	instance_classes=instance_classes,
	truth_path=truth_path,
	),
	)
	logging.info(f"Scoring {crop_name}/{label_name}...")
	truth_path = UPath(truth_path)
	# Load the predicted and ground truth label volumes
	truth_label_path = (truth_path / crop_name / label_name).path
	try:
	truth_label_ds = zarr.open(truth_label_path, mode="r")
	truth_label = truth_label_ds[:]
	except Exception:
	raise ValueError(
	f"Failed to load ground truth data for {crop_name}/{label_name}. Please contact the challenge organizers."
	)
	crop = TEST_CROPS_DICT[int(crop_name.removeprefix("crop")), label_name]
	try:
	pred_label = match_crop_space(
	pred_label_path,
	label_name,
	crop.voxel_size,
	crop.shape,
	crop.translation,
	)
	except Exception:
	raise ValueError(
	f"Failed to process submission data for {crop_name}/{label_name}. Please verify your data format and coordinate transformations are correct."

[Copilot]

The exception handler should re-raise the exception instead of catching and re-raising a new ValueError. When unzip_file already raises a ValueError with a specific message (lines 1171-1177), this handler catches it and replaces it with a less specific message. Either let the exception propagate naturally or catch specific exception types without replacing the message.

Suggested change

	except Exception:
	raise ValueError(
	"Failed to process submission file. Please ensure you submitted a valid .zip file containing a Zarr structure."
	)
	except ValueError:
	# Preserve detailed error messages raised by unzip_file
	raise
	except zipfile.BadZipFile as exc:
	# Provide a user-friendly error if the zip itself is invalid
	raise ValueError(
	"Failed to process submission file. Please ensure you submitted a valid .zip file containing a Zarr structure."
	) from exc