Skip to content

add try except for lazy loading zarr. raise informative error messsage#129

Open
mdeshotel wants to merge 1 commit intodevelopmentfrom
zarr_read_error_handling
Open

add try except for lazy loading zarr. raise informative error messsage#129
mdeshotel wants to merge 1 commit intodevelopmentfrom
zarr_read_error_handling

Conversation

@mdeshotel
Copy link
Copy Markdown

This PR adds try and except statements around s3 zarr reads and provides a more descriptive error message letting the user know that there is an issue reading the s3 zarr and that there may be an issue with the s3 data.

Additions

-Try and except statements around s3 zarr reads.

Removals

Changes

Testing

  1. Ran run suite for calibration simulations.

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Linux

@mdeshotel mdeshotel requested review from kyle-larkin and mxkpp March 30, 2026 12:34
Copy link
Copy Markdown

@mxkpp mxkpp left a comment

Choose a reason for hiding this comment

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

Comment applies throughout

return CRS(xr.open_zarr(ObjectStore(object_store)).rio.crs)
except Exception as e:
raise ValueError(
f"Unable to open zarr: {self.url}. Check that the zarr data on s3 is valid. Error: {e}"
Copy link
Copy Markdown

@mxkpp mxkpp Apr 2, 2026

Choose a reason for hiding this comment

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

Error message needs to be improved. self.url is a method not a string. Also in this case, the error could be related to the CRS() call. Please revisit each of them.

Exception handling should be improved too. ValueError is not right, and should preserve the original traceback. Something like this.

except Exception as e:
    raise RuntimeError(f"foobar. Error {e}") from e

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