Skip to content

polygonize: document that column_name is also used for geojson output#3309

Open
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-api-consistency-polygonize-2026-06-12-01
Open

polygonize: document that column_name is also used for geojson output#3309
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-api-consistency-polygonize-2026-06-12-01

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3306

  • The column_name docstring said the parameter was only used when return_type is "geopandas" or "spatialpandas". The geojson path uses it too: _to_geojson writes it as the property key on every feature. The docstring now says so.
  • Added test_polygonize_geojson_column_name to pin the geojson property naming, so the docs and the behavior can't drift apart again.

Docs-only change to the function body; no backend code touched (numpy / cupy / dask+numpy / dask+cupy all share the same return_type dispatch).

Test plan:

  • pytest xrspatial/tests/test_polygonize.py -k "geojson or invalid_return_type" (11 passed)

Found by the api-consistency sweep against the polygonize module.

…#3306)

The column_name docstring claimed it was only used for the geopandas
and spatialpandas return types, but _to_geojson uses it as the
property key on every feature. Correct the docstring and pin the
geojson property naming with a test.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 12, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the docstring fix and test.

Blockers: none.

Suggestions: none. The new wording matches what _to_geojson actually does (property key per feature), and the test pins both the key and the values so the docs can't silently drift again.

Nits:

  • The Returns section's geojson bullet ("dict representing a GeoJSON FeatureCollection") could also name column_name as the property key, but the parameter doc now covers it, so this is optional.

Checked: _to_geopandas / _to_spatialpandas / _to_geojson call sites all consume column_name; "awkward" and "numpy" do not, consistent with the new wording. Test values [0, 1, 4] match the existing test_polygonize_geojson expectations for the same fixture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

polygonize: column_name docstring omits the geojson return type

1 participant