Skip to content

Implement MRMS review feedback: explicit source metadata, product fallbacks, and robust pre-v12 band selection#480

Merged
aldenks merged 3 commits intoclaude/implement-noaa-mrms-conus-oYYudfrom
copilot/implement-pr-feedback-472
Mar 2, 2026
Merged

Implement MRMS review feedback: explicit source metadata, product fallbacks, and robust pre-v12 band selection#480
aldenks merged 3 commits intoclaude/implement-noaa-mrms-conus-oYYudfrom
copilot/implement-pr-feedback-472

Conversation

Copy link
Contributor

Copilot AI commented Mar 2, 2026

This addresses the latest PR feedback on MRMS CONUS hourly ingest behavior and metadata modeling. The update makes fallback product handling explicit in config/coords and hardens pre-v12 GRIB band selection to use discipline metadata instead of positional assumptions.

  • Source coord + internal attrs made explicit

    • NoaaMrmsSourceFileCoord now requires:
      • level: str
      • fallback_products: tuple[str, ...]
    • NoaaMrmsInternalAttrs now requires explicit values for:
      • mrms_product_pre_v12
      • mrms_level
    • NoaaMrmsInternalAttrs defines defaults for:
      • mrms_fallback_products_pre_v12 = ()
      • mrms_fallback_products = ()
    • Variables with non-empty fallback behavior set these fields explicitly.
  • Fallback product policy encoded in template metadata

    • precipitation_surface:
      • mrms_fallback_products = ("MultiSensor_QPE_01H_Pass1", "RadarOnly_QPE_01H")
      • mrms_fallback_products_pre_v12 = ("RadarOnly_QPE_01H",)
    • Other variables use the default empty fallback tuples.
    • Precipitation variable comment updated to document fallback behavior.
  • Download routing updated to try product × source combinations

    • Source order now follows time-based routing:
      • pre-v12: ("iowa",)
      • recent: ("s3", "ncep")
      • otherwise: ("s3",)
    • Download attempts iterate primary + fallback products until one succeeds.
  • Pre-v12 two-band files now select by GRIB discipline

    • For reader.count == 2 and pre-v12 timestamps, selection now finds the band whose GRIB_DISCIPLINE starts with 209, and asserts one exists.
    • Non-special case still asserts single-band inputs.
  • Targeted tests + template refresh

    • Added tests for fallback product attempt ordering and discipline-based pre-v12 band selection.
    • Updated existing tests for explicit level/fallback_products.
    • Regenerated checked-in template metadata under templates/latest.zarr.
products = (coord.product, *coord.fallback_products)
for product in products:
    for source in sources:
        try:
            return self._download_from_source(replace(coord, product=product), source=source)
        except FileNotFoundError:
            ...

[!WARNING]


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: aldenks <463484+aldenks@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement PR feedback for pull request 472 Implement MRMS review feedback: explicit source metadata, product fallbacks, and robust pre-v12 band selection Mar 2, 2026
Comment on lines 251 to 252
mrms_fallback_products_pre_v12=(),
mrms_fallback_products=(),
Copy link
Member

Choose a reason for hiding this comment

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

default these source file coord attrs to () and remove setting it explicitly to ()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 19b2f7d: mrms_fallback_products_pre_v12 and mrms_fallback_products now default to (), and I removed explicit =() assignments from the vars that were setting empties.

Co-authored-by: aldenks <463484+aldenks@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 2, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • mirrors.aliyun.com
    • Triggering command: /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek hook-impl --hook-dir /home/REDACTED/work/reformatters/reformatters/.git/hooks --script-version 4 --hook-type=pre-commit -- (dns block)
  • mirrors.cloud.tencent.com
    • Triggering command: /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek hook-impl --hook-dir /home/REDACTED/work/reformatters/reformatters/.git/hooks --script-version 4 --hook-type=pre-commit -- (dns block)
  • pypi.tuna.tsinghua.edu.cn
    • Triggering command: /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek /home/REDACTED/work/reformatters/reformatters/.venv/bin/prek hook-impl --hook-dir /home/REDACTED/work/reformatters/reformatters/.git/hooks --script-version 4 --hook-type=pre-commit -- (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@aldenks aldenks marked this pull request as ready for review March 2, 2026 15:42
@aldenks aldenks merged commit c6756a3 into claude/implement-noaa-mrms-conus-oYYud Mar 2, 2026
2 checks passed
@aldenks aldenks deleted the copilot/implement-pr-feedback-472 branch March 2, 2026 15:44
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