Repository navigation
Freeze the public surface for 1.0.0: export the README-only functions, add __all__, pin the matrix to it (#2930) - #3765
Open
brendancol wants to merge 4 commits into
Open
brendancol wants to merge 4 commits into
brendancol wants to merge 4 commits into
Conversation
…l__, and pin the matrix to it (#2930)
brendancol
commented
Nov 6, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: Freeze the public surface for 1.0.0 (#2930)
Blockers (must fix before merge)
- none
Suggestions (should fix, not blocking)
-
xrspatial/tests/test_public_surface.py:11resolves README.md relative to the package and errors if it is missing.pip install xarray-spatialthenxrspatial.test()runs from site-packages where there is no README, so this should skip rather than fail with a FileNotFoundError. -
README.md, new Hypsometric Integral row: the Source cell says "Strahler 1952", but the function docstring inxrspatial/zonal.pycites Hengl's ILWIS area-numbering page and nothing else. Do not invent a citation; use the docstring's reference or a plain "Standard".
Nits (optional improvements)
-
README.md, newdiagnoserow: the description talks about which tools a raster is "ready for". The docstring (xrspatial/diagnostics.py:113) says it runs heuristic pitfall checks, optionally narrowed withtool=. Match the docstring. -
README.md, newsuggest_zonal_canvasrow: it returns(height, width)in pixels for a given smallest polygon area andmin_pixels; "extent and cell size" is not what it returns. -
README.md, newgreat_circle_distancerow: "in meters" only holds for the defaultradius=6378137. Say the unit followsradius. - The
.xrsaccessor has no methods for the newly exportedhotspots,focal_stats,focal_apply,worley,gci,ndmi,true_color(grep ofxrspatial/accessor.py). Not for this PR, but the LTS item in #2930 declares the accessor stable, so an accessor-vs-__all__parity check belongs in a follow-up.
What looks good
- The parity test fails in both directions (checked by removing
slopefrom__all__), and skipsxrspatial/experimental/rows on purpose with a comment. - Backtick overrides keep the README readable instead of renaming rows to snake_case.
- No numeric code touched; the 647 focal/multispectral/accessor tests still pass.
Checklist
- Algorithm matches reference/paper (n/a, no numeric change)
- All implemented backends produce consistent results (n/a)
- NaN handling is correct (n/a)
- Edge cases are covered by tests (mutation check on the parity test)
- Dask chunk boundaries handled correctly (n/a)
- No premature materialization or unnecessary copies
- Benchmark exists or is not needed (watershed benchmark updated to
basin) - README feature matrix updated
- Docstrings present and accurate (README descriptions above need to match them)
…ity test outside a checkout (#2930)
brendancol
commented
Nov 6, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: follow-up after afce0ac
All findings from the first pass are addressed in afce0acf:
test_public_surface.pynow skips when README.md is absent (installed-package case). Fixed.- Hypsometric Integral row: the invented "Strahler 1952" citation is gone; Source is "Standard" and the description is the docstring's formula. Fixed.
diagnose,suggest_zonal_canvas, andgreat_circle_distancerow text now matches each docstring. Fixed.- Accessor parity with
__all__: deferred as noted, belongs with the LTS item on #2930 rather than this PR.
Nothing new. Parity test and flake8 pass on the touched files.
This was referenced Nov 6, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Nov 6, 2026
Closed
Closed
Closed
Closed
Closed
Closed
This branch has not been deployed
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.
Part of #2930. It does not close the epic: the LTS wording and the deprecated-alias removals land in separate PRs.
What changed
xrspatial/__init__.pynow exports the functions the README matrix listed but you could only reach through a submodule import:gci,ebbi,nbr2,ndmi,true_color,hotspots,focal_stats,worley, andfocal.applyasfocal_apply(same spelling rule aszonal_apply). The deprecated top-levelbasinsalias is gone;basinstays._check_numpy_versionno longer leaks into the namespace.__all__with 148 names. Withxrspatial.mcdaand the.xrsaccessor, that is the surface the stability policy calls LTS.diagnose,list_templates,hypsometric_integral,suggest_zonal_canvas, and the three scalar distance helpers), all advanced.validatedrops from stable to advanced since it has no notebook. Rows whose label is not the function name now show the exported name in backticks, e.g.focal_apply,zonal_stats,morph_erode. The two "Apply" rows and the two "Standardize" rows are told apart, and the MCDA table says those functions live inxrspatial.mcda.xrspatial/tests/test_public_surface.pyparses the matrix and checks it against__all__in both directions, with MCDA rows checked againstxrspatial.mcda.__all__. Exclusions:LANDFORM_CLASSESandmcda. Rows underxrspatial/experimental/are skipped.min_observable_heightnow says it is experimental and outside LTS.worleyandhypsometric_integralare in the reference autosummaries. The watershed benchmark callsbasin.No numeric code changed, so backend coverage is unaffected.
Test plan
pytest xrspatial/tests/test_public_surface.pypasses, and fails when a name is removed from__all__pytest xrspatial/tests/test_multispectral.py test_focal.py test_accessor.py: 647 passed