perlin: document name param, float-dtype requirement, and ValueError#3467
Merged
Conversation
The perlin() docstring listed only agg, freq, and seed in Parameters, omitting the name parameter. It also said nothing about the float-dtype requirement that the function enforces, nor the ValueError it raises on integer input (guarded by test_perlin_rejects_integer_dtype, regression from #1232). Documents name, notes the float-dtype requirement on agg, and adds a Raises section. Docstring-only; no behavior change. Records the documentation sweep state for perlin.
brendancol
commented
Jun 23, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Review: perlin docstring fix
Documentation-only change. The diff edits the perlin() docstring and adds a sweep-state CSV row. No source behavior changes.
Blockers
None.
Suggestions
None.
Nits
None.
What looks good
- The new Parameters entry for
namematches the signature default ('perlin') and the actual use at the return site. - The float-dtype note and Raises section match the real guard in perlin() and the contract codified by test_perlin_rejects_integer_dtype (regression from #1232).
- The docstring example still reproduces its documented output array; the perlin test suite passes (12/12).
- Every signature parameter except agg now appears in Parameters.
Checklist
- Algorithm matches reference/paper: n/a (no code change)
- All implemented backends produce consistent results: n/a (no code change)
- NaN handling correct: n/a
- Edge cases covered by tests: existing coverage unchanged and passing
- Dask chunk boundaries handled correctly: n/a
- No premature materialization: n/a
- Benchmark exists or not needed: not needed
- README feature matrix updated: not applicable (no new function, no backend change)
- Docstrings present and accurate: yes, this PR fixes the drift
…ion-perlin-2026-06-23
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.
Documentation-only fix for
perlin()docstring drift found by the documentation sweep.The docstring listed only
agg,freq, andseedin Parameters, but the signature isperlin(agg, freq=(1, 1), seed=5, name='perlin'). It also said nothing about the float-dtype requirement the function enforces.nameparameter (sets the output DataArray's name).aggthat input must be a floating-point dtype.ValueErroron non-float input. This guards the regression from perlin() silently corrupts int-dtyped input rasters (all pixels -> INT_MIN) #1232 (integer input corrupted every pixel to INT_MIN) and is covered bytest_perlin_rejects_integer_dtype.No behavior change. Only the docstring was edited.
Backends: not applicable (no code path touched; all four backends unchanged).
Test plan:
pytest xrspatial/tests/test_perlin.py-> 12 passedaggnow appears in Parameters; Raises section presentNote: the upstream repo has GitHub issues disabled, so there is no linked issue number.