#122 Correction Package Refactor — Complete Redesign

[mmaclay]

Copilot Summary:

This pull request introduces several improvements and refactors to the curryer.correction package, focusing on clarifying the public API, enhancing configuration and validation, and improving the geolocation error statistics pipeline. The changes include a comprehensive docstring and explicit exports in the package __init__.py, new and updated configuration dataclasses, a new Pydantic-based RegridConfig for grid handling, and a significant refactor and rename of the geolocation error statistics module to be more generic and mission-agnostic.

API and Documentation Improvements

• Added a comprehensive module-level docstring to curryer/correction/__init__.py, describing the purpose, sub-module layout, and public API of the correction package. Explicitly re-exported key classes and functions at the package level and updated the __all__ list for clear public API exposure.

Configuration and Data Structures

• Introduced PSFSamplingConfig and RegridConfig dataclasses in data_structures.py:
  • PSFSamplingConfig provides configuration for PSF sampling during image matching.
  • RegridConfig is a new Pydantic model for specifying output grid parameters, with field and model validators for consistency and error checking. [1] [2]
• Updated imports to include Pydantic and removed unused Protocols.

Data I/O and Validation

• Refactored dataio.py to clarify its role as a provider of validation helpers and S3 utilities, removed unused Protocol interface definitions, and updated documentation for clarity. [1] [2] [3]
• Improved the example for science output validation to use a more generic CSV loading example.

Geolocation Error Statistics Refactor

• Renamed geolocation_error_stats.py to error_stats.py and refactored it to be mission-agnostic:
  • Updated docstrings and configuration class to clarify that pass/fail evaluation is not part of the stats processor.
  • Added compute_percent_below for custom threshold queries.
  • Made ErrorStatsConfig the new configuration class, with improved variable name mapping and documentation.
  • Used a single source of truth for the WGS84 Earth radius via curryer.compute.constants.
  • Improved parameter and return documentation throughout. [1] [2] [3]

These changes collectively make the correction pipeline more modular, user-friendly, and robust, while clarifying the public API and improving configuration validation.

References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

---

The is the PR for epic #122 - and it will track changes from the sub-issues.

Correction Package Refactor — Complete Redesign

Top-level tracking issue for a complete refactor and architectural redesign of curryer.correction.

Goals

• Move to Pydantic config models; remove ad-hoc dataclasses
• Remove user-supplied loader protocols; internalize data loading as config-driven
• Split correction.py monolith into focused, testable modules
• Add key deliverables: verification mode & GCP regridding tool
• Lay groundwork for future CurryerConfig unifying all of curryer

PR Groupings & Sub-Issues

PR 1: Foundation — Naming & Module Split

• Rename GeolocationConfig disambiguations & error_stats.py
• Break up correction.py monolith into focused modules

PR 2: Config Redesign — Pydantic, Internalize Loading, Search Strategies

• Introduce Pydantic CorrectionConfig with typed sub-models
• Remove loader protocols — internalize data loading
• Add SearchStrategy enum for deterministic parameter sweeps

PR 3: New Features — Verification & GCP Regridding

• Add verification module
• Add gcp_regrid module

PR 4: Test Cleanup & Future Planning

• Restructure test_correction/
• [ ] Tracking: extend pydantic config pattern to all of curryer

Dependency Graph

PR 1 (Foundation)
├─ Renames (unblocks everything)
└─ Module split (unblocks everything)
    │
    ▼
PR 2 (Config & Loading)
├─ Pydantic config (unblocks loader removal + search strategies)
├─ Remove protocols (depends on pydantic config)
└─ Search strategies (depends on pydantic config)
    │
    ▼
PR 3 (Features) — can start in parallel with PR 2 by stubbing config
├─ Verification module
└─ GCP regridding module
    │
    ▼
PR 4 (Cleanup)
├─ Test restructuring
└─ Future config tracking

Context

• No external users of the correction package yet — breaking changes are acceptable
• Pydantic introduced here will serve as the prototype for a future curryer-wide config system
• CLARREO-specific code stays in tests/examples only; main library must be mission-agnostic

[codecov]

Codecov Report

❌ Patch coverage is 81.77384% with 1009 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.32%. Comparing base (996cc37) to head (539ce06).

Files with missing lines	Patch %	Lines
curryer/correction/verification.py	60.08%	162 Missing and 24 partials ⚠️
curryer/correction/pipeline.py	39.86%	153 Missing and 19 partials ⚠️
curryer/correction/image_io.py	59.52%	112 Missing and 24 partials ⚠️
curryer/correction/parameters.py	61.79%	76 Missing and 26 partials ⚠️
curryer/correction/regrid.py	67.52%	64 Missing and 12 partials ⚠️
curryer/correction/config.py	81.15%	34 Missing and 25 partials ⚠️
curryer/correction/kernel_ops.py	51.57%	39 Missing and 7 partials ⚠️
tests/test_correction/clarreo/_pipeline_helpers.py	66.66%	39 Missing and 3 partials ⚠️
tests/test_correction/_synthetic_helpers.py	61.25%	30 Missing and 1 partial ⚠️
tests/test_correction/test_pipeline.py	81.34%	25 Missing ⚠️
... and 17 more

❌ Your project check has failed because the head coverage (87.11%) is below the target coverage (95.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   73.62%   77.32%   +3.69%     
==========================================
  Files          67       94      +27     
  Lines       10655    13432    +2777     
  Branches     1204     1407     +203     
==========================================
+ Hits         7845    10386    +2541     
- Misses       2343     2525     +182     
- Partials      467      521      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR completes a broad refactor/redesign of curryer.correction, introducing a clearer public API surface, new configuration/data models (including Pydantic), expanded NetCDF result/structured-result support, and new documentation/examples/tests around verification and GCP-chip regridding.

Changes:

• Added/updated correction package modules for structured results, NetCDF I/O, unified path resolution (incl. optional S3), pairing improvements, and PSF sampling config separation.
• Added substantial pytest coverage for pipeline helpers, regridding, IO utilities, kernel ops, and CLARREO integration fixtures/helpers.
• Added user-facing docs/examples for correction/verification and GCP chip regridding; bumped package version and dependencies (Pydantic v2).

Reviewed changes

Copilot reviewed 57 out of 60 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_correction/test_results_io.py	New unit tests for NetCDF structure + checkpoint save/load/cleanup.
tests/test_correction/test_regrid.py	New unit tests for regridding primitives and end-to-end regrid flow.
tests/test_correction/test_pipeline.py	New tests for pipeline helper functions and position-column extraction behavior.
tests/test_correction/test_pairing.py	Migrates pairing tests from unittest to pytest and simplifies fixtures/helpers.
tests/test_correction/test_kernel_ops.py	New tests for offset application and calibration loading; adds extra-marked dynamic kernel test.
tests/test_correction/test_io.py	New tests for resolve_path including S3 download behaviors and cleanup.
tests/test_correction/conftest.py	Simplifies fixtures and centralizes sys.path manipulation for CLARREO test helpers.
tests/test_correction/clarreo/test_clarreo_e2e.py	Adds CLARREO end-to-end integration tests covering upstream/downstream workflows.
tests/test_correction/clarreo/test_clarreo_dataio.py	Adds AWS-credential-gated integration tests for S3 NetCDF discovery helpers.
tests/test_correction/clarreo/test_clarreo_config.py	Adds tests for CLARREO config generation + JSON round-trip validation.
tests/test_correction/clarreo/conftest.py	Adds session fixtures for CLARREO test data directories.
tests/test_correction/clarreo/clarreo_data_loaders.py	Adds CLARREO preprocessing helpers as test fixtures; keeps loader-shim behavior.
tests/test_correction/clarreo/clarreo_config.py	Updates CLARREO test fixture config factory docs/behavior to match config-driven loading.
tests/test_correction/clarreo/_pipeline_helpers.py	Adds reusable upstream/downstream pipeline runners for integration tests.
tests/test_correction/clarreo/_image_match_helpers.py	Adds discovery + error-variation helpers for deterministic image-match integration testing.
tests/test_correction/clarreo/init.py	Marks CLARREO correction tests as a package.
tests/test_correction/clarreo_data_loaders.py	Removes old top-level CLARREO loader module in favor of clarreo/ package.
tests/test_correction/_synthetic_helpers.py	Adds synthetic pairing/matching + spacecraft-state generators for upstream testing.
tests/integration/init.py	Adds integration test package marker.
pyproject.toml	Bumps version to 0.4.0b1, updates authors, adds pydantic>=2.0, sets Beta classifier.
examples/correction/regrid_gcp_chips.py	Adds a CLI-like batch regridding script for HDF GCP chips to NetCDF.
examples/correction/README.md	Adds examples index and quickstart guidance for correction/verification/regridding.
examples/correction/example_run_correction.py	Adds a correction-loop template script (intended dry-run when prerequisites missing).
examples/correction/example_config.json	Adds a generic JSON template for correction configuration.
examples/correction/clarreo_config.py	Adds a user-facing CLARREO config factory using public API models.
examples/correction/clarreo_config.json	Adds a loadable CLARREO JSON config example.
docs/source/users.md	Updates headings and adds correction-package entry-point documentation + links.
docs/source/spice_path_handling.md	Adds detailed documentation for SPICE path-shortening strategies and configuration.
docs/source/gcp_regridding.md	Adds end-user documentation for GCP chip regridding workflows and CLI usage.
docs/source/contents.rst	Updates docs to include correction user guide and adjusts acknowledgement filename.
docs/source/changelog.md	Adds 0.4.0b1 changelog entry summarizing the redesign and breaking changes.
curryer/correction/results.py	Introduces structured Pydantic result models and summary-table builder for correction runs.
curryer/correction/results_io.py	Adds config-driven NetCDF initialization, checkpointing, load/resume, and final save helpers.
curryer/correction/psf.py	Switches motion convolution config typing from GeolocationConfig to PSFSamplingConfig.
curryer/correction/pairing.py	Removes old Protocol interface, expands file pairing to .mat + NetCDF, adds in-memory dataset pairing helper.
curryer/correction/kernel_ops.py	Centralizes offset application and kernel generation utilities with improved logging.
curryer/correction/io.py	Adds unified local/S3 path resolution with temp-file tracking and atexit cleanup.
curryer/correction/image_match.py	Removes Protocol + legacy mat-loading utilities; uses PSFSamplingConfig and keeps output validation.
curryer/correction/dataio.py	Removes loader Protocols and clarifies module focus on validation + S3 utilities.
curryer/correction/data_structures.py	Introduces PSFSamplingConfig rename and new Pydantic RegridConfig with validation.
curryer/correction/init.py	Adds package-level docstring, explicit re-exports, and a curated __all__ public API.

Comments suppressed due to low confidence (1)

curryer/correction/data_structures.py:125

• PSFSamplingConfig.motion_convolution_step_m is computed from the class default gcp_step_m (30.0) at definition time, so if a caller overrides gcp_step_m the motion_convolution_step_m default will not update accordingly. Consider making motion_convolution_step_m optional and computing it in post_init when not provided (or use a default_factory).

    gcp_step_m: float = 30.0
    motion_convolution_step_m: float = gcp_step_m / 20.0
    psf_lat_sample_dist_deg: float = 2.4397105613972e-05
    psf_lon_sample_dist_deg: float = 2.8737038710207e-05

---

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

[medley56]

Without really understanding all the algorithms, I mostly reviewed for style and factoring. My overarching question is which of these functions actually need to be exposed to users? If we're trying to reduce the amount of stuff users have to actually know about, it might be wise to only expose the API that you want them to actually use, rather than exporting everything in the nested correction/* module.

I'm approving. Feel free to re-request a review if you want me to look again.

[medley56]

This is really a question of convention, but do you really want to export all the underlying functions and classes at the correction module level? I tend to shy away from this unless there is a clear user-facing API reason to do so. If this is the Curryer standard practice, then it's fine, but otherwise worth thinking about.

[medley56]

This doesn't look like an end-to-end test. It looks like it's testing each piece separately. No?

[medley56]

You can get better mocking of AWS resources using the moto library.

[medley56]

Looks like an LLM left a reference to your prompt in here. I'd recommend running a check over the changes to see if there are any other artifacts left over from prompting.

[medley56]

Is this the version you want to push?

[medley56]

I don't see this logger being used and I'd recommend against logging in tests anyway, unless you are actually examining the content of the logs as part of the test.

[medley56]

This logger I see actually being used, but again, unless you actually care about logging contents as part of the test, I personally avoid logging in tests. You might want to see where else you're instantiating loggers in test modules to make sure you really want them.

[medley56]

I'm a little confused about this module. The top line says it's a fixture but these just look like functions? Examples?

[medley56]

I usually write this kind of thing as a pytest fixture that returns a function, but maybe that's overkill for these helper functions. Just be careful about import paths when using them. It's a little confusing/fragile if your tests are doing imports from helper modules that live in unexpected places.

[medley56]

This looks like it contains a lot of Configs but they don't live in config.py. Are these a substantially different domain? Should this be data_structure_config.py?