Skip to content

geotiff: backend-parity tests for bool nodata rejection (#1921)#1924

Merged
brendancol merged 2 commits into
mainfrom
deep-sweep-test-coverage-geotiff-2026-05-15-ac4d02
May 15, 2026
Merged

geotiff: backend-parity tests for bool nodata rejection (#1921)#1924
brendancol merged 2 commits into
mainfrom
deep-sweep-test-coverage-geotiff-2026-05-15-ac4d02

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Test-only PR; source fix tracked in #1921. Found by /sweep-test-coverage pass 15.

Test plan

  • pytest xrspatial/geotiff/tests/test_write_vrt_bool_nodata_1921.py -> 13 passed, 4 xfailed locally
  • pytest xrspatial/geotiff/tests/test_nodata_bool_rejection_1911.py still green (no overlap regression)

Copilot AI review requested due to automatic review settings May 15, 2026 14:17
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds test coverage to ensure GeoTIFF/VRT writer entry points consistently reject nodata=True/False (and np.bool_) by raising TypeError, and records the sweep-test-coverage state update for issue #1921 tracking.

Changes:

  • Add new backend-parity tests covering write_vrt bool-nodata behavior (including strict xfails for the expected future fix).
  • Add GPU-path tests pinning bool-nodata rejection for direct write_geotiff_gpu calls and the to_geotiff(gpu=True) dispatch path.
  • Update .claude/sweep-test-coverage-state.csv to record sweep pass 15 / issue #1921 metadata.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
xrspatial/geotiff/tests/test_write_vrt_bool_nodata_1921.py Introduces new parity tests for bool/np.bool_ nodata rejection across write_vrt and GPU writer paths.
.claude/sweep-test-coverage-state.csv Updates sweep-test-coverage tracking state for geotiff module / issue #1921.
Comments suppressed due to low confidence (1)

xrspatial/geotiff/tests/test_write_vrt_bool_nodata_1921.py:145

  • These tests should be skipped with the repo-standard GPU marker (reason "cupy + CUDA required") rather than only checking _HAS_CUPY with reason "cupy not available". As written, they may run (and fail) when CuPy is installed but CUDA is unavailable; consider replacing with @requires_gpu from geotiff/tests/conftest.py.
@pytest.mark.skipif(not _HAS_CUPY, reason="cupy not available")
@pytest.mark.parametrize(
    "bad",
    [True, False, np.bool_(True), np.bool_(False)],
)
def test_write_geotiff_gpu_rejects_bool_nodata(uint8_da, tmp_path, bad):
    """Direct ``write_geotiff_gpu`` call rejects bool nodata.

    Currently raises ``TypeError`` only because the deeper
    ``build_geo_tags`` guard fires. Pinning the behaviour so a refactor
    that drops the deeper guard surfaces here, not in user code.
    """
    from xrspatial.geotiff import write_geotiff_gpu
    path = str(tmp_path / "gpu_1921_bad.tif")
    with pytest.raises(TypeError, match="nodata must be numeric"):
        write_geotiff_gpu(uint8_da, path, nodata=bad)


@pytest.mark.skipif(not _HAS_CUPY, reason="cupy not available")
def test_to_geotiff_gpu_dispatch_rejects_bool_nodata(uint8_da, tmp_path):

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

from __future__ import annotations

import os
import tempfile


def test_write_vrt_with_bool_nodata_currently_emits_string(
src_geotiff, tmp_path):
Comment on lines +33 to +34
import cupy # noqa: F401
_HAS_CUPY = True
Pin two parity gaps the #1911 fix left open:

* write_vrt(nodata=True) currently writes <NoDataValue>True</NoDataValue>
  into the VRT XML. Four xfail(strict=True) tests cover the fix surface
  and one passing test pins the buggy emission so the source fix shows
  up as a diff. Tracked in issue #1921.
* write_geotiff_gpu direct call rejects bool nodata only via the deeper
  build_geo_tags guard. Add a top-of-call parity test so a refactor that
  drops the deeper check surfaces here, not in user code.

Also pin to_geotiff(gpu=True, nodata=True) so the dispatch order keeps
firing the eager guard before reaching the GPU writer.

Found by /sweep-test-coverage pass 15.
- Remove unused tempfile import (F401)
- Switch GPU gating from local _HAS_CUPY import check to
  requires_gpu from conftest, which also verifies the CUDA
  runtime is usable
@brendancol brendancol force-pushed the deep-sweep-test-coverage-geotiff-2026-05-15-ac4d02 branch from 45593cd to 4b3cc92 Compare May 15, 2026 14:37
@brendancol brendancol merged commit 75a162d into main May 15, 2026
10 of 11 checks passed
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.

2 participants