Uh oh!
There was an error while loading. Please reload this page.
Remove orphaned vision page and correct roadmap claims - #692
Merged
Conversation
docs/user-guide/vision.qmd was not in the Quarto sidebar or navbar and nothing linked to it, but Quarto still rendered it and indexed it in site search — so users could land on a page stating forecast lead time was "Planned 2024". Its content is superseded by roadmap/: Limitations by not-planned.md, and the Future section by forecast-lead-time.md, web-app.md and automatic-reports.md. Roadmap corrections: - baseline-comparisons.md claimed a persistence baseline and climatological baselines were delivered in v1.3. Neither exists; DummyModelResult supports only 'mean' and 'constant', and it first shipped in v1.1. Scope the entry to what exists and record the two missing baselines explicitly, noting that persistence is lead-time dependent. - vertical-profiles.md had frontmatter "In Development" contradicting a body saying "Planned for a future release. Design work is needed", when VerticalObservation, VerticalModelResult and SkillProfile all exist. Describe the actual alpha state. - network-models.md advertised matching at catchments, which is not supported, and omitted that res1d reading needs the optional networks extra.
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
Changes are documentation-only and align published roadmap claims with verified existing functionality, with only minor wording/copy-paste improvements suggested.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR cleans up the documentation/roadmap by removing an orphaned Quarto page that could still be indexed via site search, and by correcting roadmap feature descriptions to match what ModelSkill actually supports today (notably baseline comparisons, vertical profiles, and network model support).
Changes:
- Remove
docs/user-guide/vision.qmd, whose content is superseded by theroadmap/feature pages but could still be discovered via search. - Correct roadmap language for baseline comparisons to reflect the existing
DummyModelResultstrategies (mean,constant) and explicitly note missing persistence/climatology baselines. - Align vertical profile and network model roadmap entries with current (alpha/in-development) implementation details and installation requirements.
File summaries
| File | Description |
|---|---|
| roadmap/README.md | Regenerated roadmap overview reflecting corrected feature summaries. |
| roadmap/features/baseline-comparisons.md | Updates claims to match implemented baseline strategies; documents non-included baselines. |
| roadmap/features/vertical-profiles.md | Rewrites current status to reflect existing vertical classes and alpha stability. |
| roadmap/features/network-models.md | Removes unsupported catchment matching claim; documents optional networks install requirement. |
| docs/user-guide/vision.qmd | Removes an unreachable-but-searchable page with outdated roadmap claims. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+24
to
+26
| In active development, currently at an alpha stage. `VerticalObservation` and | ||
| `VerticalModelResult` exist, along with depth-resolved skill (`SkillProfile`) and profile | ||
| plotting. The surface is not yet stable and is subject to change before release. |
Comment on lines
+24
to
+26
| In active development. Reading of MIKE 1D result files is already supported via | ||
| `Network.from_res1d`, which requires the optional `networks` dependency group | ||
| (`pip install modelskill[networks]`). Integration with ModelSkill's validation workflow is underway. |
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 freeto 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.
Summary
docs/user-guide/vision.qmdwas unreachable through the docs navigation — absent from the Quarto sidebar and navbar, with nothing linking to it — but Quarto rendered it anyway and indexed it in site search. A user searching the docs could land on a page stating that forecast lead time was "Planned 2024". Its content is fully superseded byroadmap/: the Limitations section bynot-planned.md, and the Future section byforecast-lead-time.md,web-app.mdandautomatic-reports.md.Three roadmap entries also made claims the code does not support:
baseline-comparisons.mdadvertised a persistence baseline and climatological baselines as "Delivered in v1.3". Neither exists —DummyModelResultsupports onlymeanandconstant, and it first shipped in v1.1. The entry now describes what exists and records the two missing baselines under "Not Included", noting that a persistence baseline is lead-time dependent and so depends on forecast lead-time support.vertical-profiles.mdhad frontmatterstatus: "In Development"contradicting a body that said "Planned for a future release. Design work is needed", whileVerticalObservation,VerticalModelResultandSkillProfileall exist. Now describes the actual alpha state.network-models.mdadvertised matching at catchments, which is unsupported, and omitted that res1d reading requires the optionalnetworksextra.roadmap/README.mdis regenerated from the frontmatter.Checked and left alone:
custom-metrics.md(the@metricdecorator did first ship in v1.2.0) andskill-aggregation.md(seasonal grouping is available viafreq:QS-DEC,dt:quarter, or a user-supplied category column, even though there is nodt:seasonaccessor).