Repository navigation
feat(spatial): let readPolygon reuse a pre-resolved selection - #21
Conversation
readPolygon re-scanned the bbox (resolveSelection -> gatherCells, O(rows*cols)) on every call. A caller reading many same-shape variables through one polygon paid that scan per variable. Add an optional resolvedSelection so the scan runs once (resolvePolygonCells) and is threaded into every read. Additive/back-compat; GridIndex reuse via spatialLayout is unchanged.
readPolygon consumed a caller-supplied resolvedSelection verbatim with no validation against the array's shape. A selection resolved for a differently-shaped array would slice out of range and silently yield fill/NaN values (arr.get pads the out-of-range region) instead of erroring. Add assertSelectionInBounds: validate the bbox against the resolver's spatial extents and each cell against its own bbox, throwing SliceError on mismatch. Also correct the resolvedSelection JSDoc (polygon is needed only for validation; timeRange is applied fresh per read; maxCells has no effect but must still be valid) and add tests for both guard paths.
There was a problem hiding this comment.
Pull request overview
This PR extends the spatial polygon reader so callers can reuse a previously resolved polygon cell selection across multiple readPolygon calls, avoiding repeated O(bbox) membership scans when reading many same-shape variables through the same polygon.
Changes:
- Adds optional
resolvedSelection?: PolygonSelectiontoPolygonReadOptionsand updatesreadPolygonto reuse it when provided. - Introduces a new bounds-check guard for caller-supplied selections (intended to fail fast on common misuse cases).
- Adds unit tests covering equivalence (inline vs reused), verbatim selection trust, empty selection behavior, and guard error cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/spatial/polygon-reader.ts |
Adds resolvedSelection option, guard logic, and updates readPolygon flow to reuse pre-resolved selections. |
tests/unit/polygon-reader.test.ts |
Adds test coverage for reusing resolvedSelection and for guard behavior on invalid selections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| rMin < 0 || | ||
| cMin < 0 || | ||
| rMax > resolver.nRows || | ||
| cMax > resolver.nCols || |
There was a problem hiding this comment.
Good catch — fixed in bff07fc. assertSelectionInBounds now keys off arr's actual trailing spatial extents (arr.shape[rank-2]/[rank-1] for 3-D [time, rows, cols]; the single trailing point axis for 2-D [time, npoints]) instead of the resolver's layout-derived nRows/nCols. A selection resolved for a different-shaped array now errors even when the caller passes a matching spatialLayout. Added a regression test: selection fits a 5×5 grid layout but is reused against a 4×4 array → clean SliceError before any read.
…ayout assertSelectionInBounds validated resolvedSelection.bbox against the resolver's declared nRows/nCols, which are derived from spatialLayout (the GridIndex / lat-lon coords) rather than from arr.shape. A caller could therefore reuse a selection resolved for a different-shaped array without tripping the guard, as long as they also passed a spatialLayout that matched the selection — arr.get would then slice out of range and silently return fill/NaN, exactly what this guard exists to prevent. Key the check off arr's actual trailing spatial extents instead: - 3-D [time, rows, cols] layouts -> last two axes of arr.shape - 2-D [time, npoints] layout -> single trailing point axis Add a regression test: selection fits the spatialLayout (5x5 grid) but not the reused array (4x4) -> clean SliceError before any read.
Contexto
O path de meteograma de área da nautilus-api lê N variáveis de mesma shape através de um único polígono. A PR i4sea #1154 (#1109) já hoista o build do
GridIndexe o scan standalone do guard (resolvePolygonCells) para 1x por request — masreadPolygonre-resolve a seleção internamente a cada chamada (resolveSelection->gatherCells, O(rows*cols)), então o membership scan ainda roda por variável (speed + direction).PolygonReadOptionsnão tinha como injetar umaPolygonSelectionjá resolvida.O que muda
resolvedSelection?: PolygonSelectionemPolygonReadOptions.readPolygonusaopts.resolvedSelection ?? resolveSelection(...)— pula ogatherCellsO(bbox) quando a seleção é fornecida.resolveLayout+assertArrayRankpermanecem (a leitura per-step usa o layout resolver; são O(1)/O(rank)).resolvePolygonCellscom a MESMAarrshape/rank + mesmapolygon/spatialLayout/timeRange/selection/maxCells;maxCellsé ignorado quando setado (o stride já está embutido).Assim um caller resolve 1x e passa o resultado a cada
readPolygon, eliminando os scans residuais por variável — completa o objetivo do #1109.Compatibilidade
Aditivo e back-compat: campo opcional, comportamento inalterado quando ausente. MINOR (0.10.0 -> 0.11.0).
Testes (novos)
readPolygoninline vsresolvedSelectionreusado dá timesteps idênticos.Verificação
Todos os gates do
prepublishOnly:Próximo passo
Após publicar 0.11.0: bump em `apps/nautilus-api` + wiring no `AreaGeometryResolver` para passar a seleção a `readPolygon`/`openDirectionSteps` (PR i4sea separada).