Skip to content

Allow tiny round-off exceedence of max_resolution in spatial check#30

Merged
leifdenby merged 3 commits into
mlcast-community:mainfrom
ladc:29-fix-spatial-check
Mar 4, 2026
Merged

Allow tiny round-off exceedence of max_resolution in spatial check#30
leifdenby merged 3 commits into
mlcast-community:mainfrom
ladc:29-fix-spatial-check

Conversation

@ladc

@ladc ladc commented Feb 22, 2026

Copy link
Copy Markdown
Member

Describe your changes

Make checks on max spatial resolution (1km) more lenient.

The be-rmi dataset was not passing due to this criterion for @sidekock.

Using math.isclose as suggested by @leifdenby

Closes #29

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality, e.g. adding a new dataset)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected, e.g. removing or moving a dataset in the catalog)
  • 📖 Documentation (Addition or improvements to documentation)
  • 🔨 Maintenance (Fixing issues with CI/CD and changes to upstream dependencies)

Checklist before requesting a review

  • My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
  • I have performed a self-review of my code
  • For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
  • I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
  • I have updated the documentation to cover introduced code changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have given the PR a name that clearly describes the change, written in imperative form (context).
  • I have requested a reviewer and an assignee (assignee is responsible for merging)

Checklist for reviewers

Each PR comes with its own improvements and flaws. The reviewer should check the following:

  • the code is readable
  • the code is well tested
  • the code is documented (including return types and parameters)
  • the code is easy to maintain

Author checklist after completed review

  • I have added a line to the CHANGELOG describing this change, in a section
    reflecting type of change (add section where missing):
    • added: when you have added new functionality
    • changed: when default behaviour of the code has been changed
    • fixes: when your contribution fixes a bug

Checklist for assignee

  • PR is up to date with the base branch
  • the tests pass
  • author has added an entry to the changelog (and designated the change as added, changed or fixed)
  • Once the PR is ready to be merged, squash commits and merge the PR.

@ladc ladc marked this pull request as draft February 22, 2026 14:25
@ladc ladc marked this pull request as ready for review February 22, 2026 14:50
@leifdenby leifdenby self-requested a review February 24, 2026 12:25

@leifdenby leifdenby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perfect! Thanks for doing this @ladc!

I just ran the validator on your branch against the DMI radar dataset and it all looks fine to me (the spatial resolution check still passes as it should):

uvx --from "git+https://github.com/ladc/mlcast-dataset-validator@29-fix-spatial-check" mlcast.validate_dataset --s3-anon --s3-endpoint-url https://object-store.os-api.cci2.ecmwf.int source_data radar_precipitation s3://mlcast-source-datasets/dmi/v0.1.0/dmi_10_minutes.zarr/

Could you add an entry to the CHANGELOG with your changes? Then this is ready to go in! Thanks again :)

@ladc ladc requested a review from leifdenby February 25, 2026 12:55

@leifdenby leifdenby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Thanks for doing this :)

@leifdenby leifdenby merged commit 74fbd74 into mlcast-community:main Mar 4, 2026
3 checks passed
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.

Spatial resolution checks are unnecessarily strict

2 participants