Feature/long term forecast integration#3
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…client Adds read_long_forecasts(), write_long_forecasts(), and prepare_long_forecast_records() targeting the /long-forecast/ endpoint. Includes input validation (horizon_type, model, skip, limit), required column checks in prepare, NaN-to-None conversion, and 12 new tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e domain classes Extract SapphirePostprocessingBase (skill metrics), SapphireShortTermForecastClient (short-term + LR forecasts), and SapphireLongTermForecastClient (long-term forecasts) from the monolithic SapphirePostprocessingClient. The original class becomes a backwards-compatible facade with deprecated aliases that emit DeprecationWarning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ast clients Cover empty DataFrames, all-NaN columns, zero/extreme values, multi-entity inputs, row order preservation, date boundaries (year transition, leap year), target param coverage, LR forecast validation, and custom column names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces long-term forecast support alongside a refactor of the postprocessing client into focused short-term/long-term clients plus a backwards-compatible facade, with expanded tests to cover the new API surface.
Changes:
- Added
SapphireShortTermForecastClientandSapphireLongTermForecastClient(plus sharedSapphirePostprocessingBase) and updatedSapphirePostprocessingClientto act as a facade with deprecated alias methods. - Added validators for long-term forecast horizon/model enums.
- Added new test suites for short-term and long-term clients; updated facade tests to focus on inheritance and deprecation warnings.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_short_term.py | New tests for short-term + LR forecast read/write and record preparation helpers. |
| tests/test_long_term.py | New tests for long-term forecast read/write and record preparation helpers. |
| tests/test_postprocessing.py | Refocused tests to validate facade inheritance, deprecated aliases, and shared skill-metric access. |
| src/sapphire_api_client/validators.py | Added enum sets for long-term horizon types and model names. |
| src/sapphire_api_client/short_term.py | New short-term + LR client implementation using postprocessing base. |
| src/sapphire_api_client/long_term.py | New long-term forecast client with query param validation and record preparation. |
| src/sapphire_api_client/postprocessing_base.py | New shared base for postprocessing clients (SERVICE_PREFIX + skill metrics). |
| src/sapphire_api_client/postprocessing.py | Converted to facade combining both forecast families and providing deprecated aliases. |
| src/sapphire_api_client/init.py | Exported the new short-term and long-term client classes from the package root. |
| CLAUDE.md | Documented updated client hierarchy and expanded testing guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+187
to
+190
| if "flag" in df.columns: | ||
| val = row.get("flag") | ||
| record["flag"] = int(val) if pd.notna(val) else None | ||
| if "composition" in df.columns: |
There was a problem hiding this comment.
prepare_long_term_forecast_records() converts flag via int(val) when present. Elsewhere in the codebase (e.g., preprocessing record builders) integer conversions use safe_int_conversion() to consistently handle NaN/None and to raise clearer conversion errors; using the shared helper here would align behavior and error messages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.