Clean up plotting.spatial_overview
- Dominant language
- Python
- Stars
- 56
- Forks
- 9
- Avg merge
- 57m
- Merged PRs (30d)
- 3
Description
[`src/modelskill/plotting/_spatial_overview.py`](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py) has accumulated debt over time. Worth a focused cleanup pass.
## Symptoms
- Five TODOs scattered across ~110 lines:
- [L71](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L71) — `support Gridded ModelResults`
- [L78](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L78) — `better support for multiple models`
- [L87](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L87) — `this is not supported for all model types`
- [L98](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L98) — `group by lonlat bin or sample randomly` (track downsampling)
- [L109](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L109) — `adjust xlim to accomodate text` (annotation overflow)
- Explicit [`raise ValueError` for `PointModelResult`, `TrackModelResult`, `VerticalModelResult`](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L73-L76) — a reject-list rather than an accept-list
- [Type hint claims `DfsuModelResult | GeometryFM2D | Iterable[...]`](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L18-L24) but duck-typing in the body accepts more
- Duck-typed adapter for `GeometryFM3D` added in #646 as a stop-gap — fine for now, but symptomatic
- Mixed control flow ([`g = m.data.geometry` / `g = m`](https://github.com/DHI/modelskill/blob/e528570be69af1057f330c8ee72640122af240db/src/modelskill/plotting/_spatial_overview.py#L77-L84)) with a stray indented comment between branches
## Suggested direction
- Make the accept list explicit: which model/geometry types does this function plot, and what does it do for each?
- Decide on `GridModelResult` support (or document the gap) — the TODO has been there for a while
- Consider whether 3D-aware behavior belongs here or in a `model.outline` helper on the model classes themselves (each type knows how to draw its own footprint)
- Type hints should match the accepted set, not the duck-typed reality
## Out of scope for the fix PR
This issue is the place to design the cleanup; the immediate notebook-test breakage is fixed in #646.
Contributor guide
Assessment
This issue has not been assessed yet.