Skip to content

Tutorial on building an accesor and site archive - #275

Open
stevehadd wants to merge 24 commits into
ACCESS-Community-Hub:developfrom
stevehadd:archive_tutorial
Open

stevehadd wants to merge 24 commits into
ACCESS-Community-Hub:developfrom
stevehadd:archive_tutorial

Conversation

@stevehadd

@stevehadd stevehadd commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

This PR includes some tutorial notebooks and a site archive for AWS data, which can be run from anywhere.
Changes:

  • Site Archive tutorial notebook
  • Acessor tutorial notebook
  • Inclusion of new notebooks in gallery
  • AWS site archive package in a new "third party" directory
    • Idea is we can include packages for specific other data sources e.g. CDS and also other integrations like mlflow or nvidia earth2 studio in future
  • tests for site archive

Getting Started

  • [ X] If there is not an existing issue, raise a new issue
  • In the issue, state that you are intending to work on a contribution. This gives everyone involved the opportunity to discuss the best way forward.

Docstrings

  • [X ] Docstrings complete and follow Napoleon (google) style

Test Coverage

  • [X ] All new code is covered by unit tests

Documentation

  • [X ] Documentation is updated as required

Final Checks:

  • [ X] If no generative AI was used, then tick this box

Alternatively, if generative AI was used, then confirm you have:

  • Attributed any generative AI (such as GitHub Copilot) that was used in this PR. See our contributing guide for more information.
  • Included the name and version of the tool or system in the pull request
  • Described the scope of that use

Finally,

  • Mark the PR as ready to review. Note - we encourage you to ask for feedback at the outset or at any time during the work.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 High severity · 2 Medium severity · 3 Low severity

Open (10)
What changed in this PR

Adds AWS-backed Met Office UKV and global 10 km accessors, tutorials, packaging metadata, and tests.

Changes:

  • Added AWS site archive accessors and registration.
  • Added tutorials and gallery entries.
  • Added requirements, packaging configuration, and integration tests.
File Description
packages/​third_party/​aws_site_archive/​tests/​test_ukv.py Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​tests/​test_moglobal.py Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​tests/​report.xml Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_ukv.py Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_global_10km.py Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​__init__.py Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​requirements.txt Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​requirements-dev.txt Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​README.md Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​pyproject.toml Updated as part of this pull request.
packages/​third_party/​aws_site_archive/​MANIFEST.in Updated as part of this pull request.
docs/​notebooks/​Gallery.ipynb Updated as part of this pull request.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/third_party/aws_site_archive/pyproject.toml Outdated
Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/mo_global_10km.py Outdated
Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/mo_ukv.py Outdated
Comment thread packages/third_party/aws_site_archive/tests/test_moglobal.py
Comment thread packages/third_party/aws_site_archive/tests/test_ukv.py
Comment thread packages/third_party/aws_site_archive/pyproject.toml Outdated
Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/__init__.py Outdated
Comment thread packages/third_party/aws_site_archive/README.md
Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/mo_global_10km.py Outdated
Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/mo_ukv.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical accessor behavior and test assertions remain unresolved, along with configuration inconsistencies.

Review effort: Lite
Findings: 6 High severity · 1 Medium severity · 3 Low severity

Open (10)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Low severity Fix cache recommendation spelling

packages/​third_party/​aws_site_archive/​README.md:5

The new documentation says it is “recommnded” to use a cache; correct the spelling in this user-facing guidance.

Low severity Correct global archive documentation URL

packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_global_10km.py:103

The global accessor's documentation URL points to the UK deterministic archive, so users following the metadata are sent to the wrong dataset documentation. Use the global deterministic archive URL instead.

Comment thread packages/third_party/aws_site_archive/tests/test_moglobal.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Moderate implementation, dependency, registration, and test reliability issues remain unresolved.

Review effort: Lite
Findings: 3 High severity · 1 Medium severity · 1 Low severity

Open (5)
Resolved since last review (6)
Previously missed (7)

In code that hasn't changed since last review

Medium severity Load configured archive roots during package initialization

packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​__init__.py:33

The README says that ~/.pyearthtoolsconfig can override these defaults, but this module never calls the imported load_root_directories_from_config (unlike site_archive_met_office). As a result, configured roots are ignored whenever this package is imported; load the config after registering the defaults.

Medium severity Normalize string variables to a one-element list

packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_global_10km.py:46

variables is documented and registered as accepting either a string or a list, and the registered .sample() path passes a string. Keeping the string here makes filesystem() iterate over its characters, so MOGlobal10km.sample() generates one invalid S3 path per character instead of one variable path. Normalize a string to a one-element list before storing it.

Medium severity Reject empty variable lists in MOGlobal accessor

packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_global_10km.py:74

Unlike MOUKV, this accessor does not reject an empty variable list. The committed test report shows test_moglobal_novar as an XPASS, so the xfail is not exercising the documented failure behavior and an empty request can proceed to an empty merge. Add the same early DataNotFoundError guard used by UKV.

Medium severity Normalize string variables to a one-element list

packages/​third_party/​aws_site_archive/​src/​site_archive_aws/​mo_ukv.py:46

variables is documented and registered as accepting either a string or a list, and the registered .sample() path passes a string. Keeping the string here makes filesystem() iterate over its characters, so MOUKV.sample() generates one invalid S3 path per character instead of one variable path. Normalize a string to a one-element list before storing it.

Medium severity Correct global archive dimensions in slow-load test

packages/​third_party/​aws_site_archive/​tests/​test_moglobal.py:63

The global archive output is not a UKV-sized grid: the site-archive notebook shows global data with dimensions (pressure=37, latitude=1920, longitude=2560), so running this marked-slow test will fail against the implementation's actual dataset shape (1, 37, 1920, 2560). Update the assertion to the global grid dimensions.

Low severity Notebook Python version is unsupported by project bounds

docs/​notebooks/​Gallery.ipynb:219

This notebook metadata now records Python 3.14.6, while the repository and the new AWS package both declare requires-python = ">=3.11, <3.14". The gallery therefore claims an execution environment that cannot install the documented project; keep the metadata on a supported Python version or update the package bounds after validating 3.14 support.

Low severity Fix misspelling in README installation guidance

packages/​third_party/​aws_site_archive/​README.md:5

The new README contains the misspelling recommnded; correct it so the installation guidance is rendered professionally.

This issue also appears on line 23 of the same file.

Comment thread packages/third_party/aws_site_archive/src/site_archive_aws/mo_global_10km.py Outdated
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