diff --git a/xrspatial/contour.py b/xrspatial/contour.py index a4c11c52f..7261015fd 100644 --- a/xrspatial/contour.py +++ b/xrspatial/contour.py @@ -586,7 +586,9 @@ def contours( n_levels : int, default 10 Number of contour levels to generate when ``levels`` is not - provided. + provided. Must be an integer >= 1; otherwise a ``TypeError`` + (non-integer) or ``ValueError`` (< 1) is raised. Ignored when + ``levels`` is given, in which case it is not validated. return_type : str, default "numpy" Output format. ``"numpy"`` returns a list of ``(level, coords)`` @@ -623,6 +625,17 @@ def contours( # Determine contour levels. if levels is None: + # n_levels is only consumed on this auto-level branch, so validate + # it here. When explicit levels are supplied n_levels is ignored, + # and rejecting it then would be surprising. Reject bools too: + # bool is an int subclass, but True/False as a level count is a bug. + if isinstance(n_levels, bool) or not isinstance(n_levels, (int, np.integer)): + raise TypeError( + f"n_levels must be an integer, got {type(n_levels).__name__}" + ) + if n_levels < 1: + raise ValueError(f"n_levels must be >= 1, got {n_levels}") + # Reduce over finite values only. +/-inf cells would otherwise # poison the range and make np.linspace emit non-finite levels, # silently dropping every contour for the finite terrain. An diff --git a/xrspatial/tests/test_contour.py b/xrspatial/tests/test_contour.py index 0066b66da..5a5829b1d 100644 --- a/xrspatial/tests/test_contour.py +++ b/xrspatial/tests/test_contour.py @@ -221,6 +221,54 @@ def test_auto_levels_ignore_inf_dask_cupy(self): assert np_levels == dc_levels +# --------------------------------------------------------------------------- +# n_levels validation contract (issue #2895) +# --------------------------------------------------------------------------- + +class TestNLevelsValidation: + + def test_n_levels_zero_raises(self): + """n_levels=0 must raise instead of silently returning nothing.""" + agg = create_test_raster(_make_ramp(), backend='numpy') + with pytest.raises(ValueError, match="n_levels must be >= 1"): + contours(agg, n_levels=0) + + def test_n_levels_negative_raises(self): + """n_levels=-1 must raise a clear out-of-range error.""" + agg = create_test_raster(_make_ramp(), backend='numpy') + with pytest.raises(ValueError, match="n_levels must be >= 1"): + contours(agg, n_levels=-1) + + def test_n_levels_float_raises_clear_typeerror(self): + """A non-integer n_levels raises a clear TypeError naming the + parameter, not a raw numpy 'cannot be interpreted as an integer'.""" + agg = create_test_raster(_make_ramp(), backend='numpy') + with pytest.raises(TypeError, match="n_levels must be an integer"): + contours(agg, n_levels=2.5) + + def test_n_levels_bool_raises(self): + """bool is an int subclass but is not a valid level count.""" + agg = create_test_raster(_make_ramp(), backend='numpy') + with pytest.raises(TypeError, match="n_levels must be an integer"): + contours(agg, n_levels=True) + + def test_n_levels_one_produces_single_level(self): + """n_levels=1 is valid and yields exactly one contour level.""" + agg = create_test_raster(_make_ramp(ny=5, nx=10), backend='numpy') + result = contours(agg, n_levels=1) + levels = sorted({lvl for lvl, _ in result}) + assert len(levels) == 1 + + def test_explicit_levels_skip_n_levels_validation(self): + """When explicit levels are given, n_levels is unused and an + otherwise-invalid value must not be rejected.""" + agg = create_test_raster(_make_ramp(ny=5, nx=6), backend='numpy') + # n_levels=0 would raise on the auto branch; with explicit levels + # it is ignored and the call must succeed. + result = contours(agg, levels=[2.5], n_levels=0) + assert len(result) > 0 + + # --------------------------------------------------------------------------- # Edge cases # ---------------------------------------------------------------------------