Only apply the cyclostrophic deprecation shim when the argument is passed

AUTHORS.md

@@ -38,3 +38,4 @@
 * Valentin Gebhart
 * Dahyann Araya
 * Giovanni Cozzolongo
+* Dylan Pulver

CHANGELOG.md

@@ -30,6 +30,7 @@ Code freeze date: YYYY-MM-DD
 - `Hazard.from_raster_xarray` now returns a sparse matrix instead of a sparse array [#1261](https://github.com/CLIMADA-project/climada_python/pull/1261).
 - `ImpactCalc.impact` now raises a clear `ValueError` when the supplied `Hazard` contains no events, instead of failing later inside `np.array_split` with an obscure message [#814](https://github.com/CLIMADA-project/climada_python/issues/814).
 - Fix TCTracks.from_FAST duplicate loading from year loop [#1269](github.com/CLIMADA-project/climada_python/pull/1269)
+- `compute_angular_windspeeds` no longer overrides `model_kwargs["cyclostrophic"]` with its own default, and no longer emits a `DeprecationWarning` when the deprecated `cyclostrophic` argument is not passed. The caller's `model_kwargs` dict is no longer modified in place. [#1209](https://github.com/CLIMADA-project/climada_python/issues/1209)
 
 ### Deprecated
 - `Impact.calc_freq_curve()` should not be given the parameter `return_per`. Use the parameter `return_periods` in `Impact.calc_freq_curve().interpolate()` instead.

climada/hazard/test/test_trop_cyclone_windfields.py

@@ -1,4 +1,5 @@
 import unittest
+import warnings
 
 import numpy as np
 import xarray as xr
@@ -9,6 +10,7 @@
     H_TO_S,
     KM_TO_M,
     MBAR_TO_PA,
+    MODEL_VANG,
     _B_holland_1980,
     _bs_holland_2008,
     _stat_er_2011,
@@ -17,6 +19,7 @@
     _v_max_s_holland_2008,
     _vtrans,
     _x_holland_2010,
+    compute_angular_windspeeds,
     get_close_centroids,
     tctrack_to_si,
 )
@@ -302,6 +305,84 @@ def test_er_2011_pass(self):
             ],
         )
 
+    def _er_2011_setup(self):
+        """Track/centroid setup shared by the ``compute_angular_windspeeds`` tests.
+
+        Only the second track node is asserted on, since ``compute_angular_windspeeds``
+        zeroes out the first one.
+        """
+        d_centr = KM_TO_M * np.array(
+            [[35, 70, 75, 220], [30, 150, 1000, 300]], dtype=float
+        )
+        si_track = xr.Dataset(
+            {
+                "rad": ("time", KM_TO_M * np.array([75.0, 40.0])),
+                "vmax": ("time", [35.0, 40.0]),
+                "lat": ("time", [20.0, 27.0]),
+                "cp": ("time", [4.98665369e-05, 6.61918149e-05]),
+            }
+        )
+        mask = np.array(
+            [[True, True, True, True], [True, False, True, True]], dtype=bool
+        )
+        return si_track, d_centr, mask
+
+    # Reference values for the second track node (r_max = 40 km, v_max = 40 m/s,
+    # f = 6.61918149e-05 1/s) from equation (36) of Emanuel and Rotunno 2011,
+    # M = M_max * 2 * (r/r_max)^2 / (1 + (r/r_max)^2) and v = M / r.
+    # Cyclostrophic: M_max = r_max * v_max = 1.6e6 m^2/s, so at r = 30 km,
+    # v = 1.6e6 * 2 * 0.5625 / 1.5625 / 30e3 = 38.4 m/s.
+    ER11_NODE1_CYCLOSTROPHIC = [38.4, 0.0, 3.194888178913738, 10.480349344978167]
+    # Non-cyclostrophic: M_max additionally contains 0.5 * f * r_max^2 = 52953.45 m^2/s.
+    ER11_NODE1_WITH_CORIOLIS = [39.670883, 0.0, 3.300626, 10.827206]
+
+    def test_compute_angular_windspeeds_cyclostrophic_model_kwarg(self):
+        """``cyclostrophic`` passed via ``model_kwargs`` must reach the wind model."""
+        si_track, d_centr, mask = self._er_2011_setup()
+        v_ang_norm = compute_angular_windspeeds(
+            si_track,
+            d_centr,
+            mask,
+            MODEL_VANG["ER11"],
+            model_kwargs={"cyclostrophic": True},
+        )
+        np.testing.assert_allclose(
+            v_ang_norm[1], self.ER11_NODE1_CYCLOSTROPHIC, atol=1e-6
+        )
+        # guard against the setting being silently dropped, which yields the
+        # Coriolis-corrected profile instead
+        self.assertFalse(
+            np.allclose(v_ang_norm[1], self.ER11_NODE1_WITH_CORIOLIS, atol=1e-4)
+        )
+
+    def test_compute_angular_windspeeds_no_spurious_deprecation(self):
+        """No DeprecationWarning unless the deprecated argument is actually passed."""
+        si_track, d_centr, mask = self._er_2011_setup()
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter("always")
+            compute_angular_windspeeds(
+                si_track, d_centr, mask, MODEL_VANG["ER11"], model_kwargs={}
+            )
+        self.assertEqual(
+            [w for w in caught if issubclass(w.category, DeprecationWarning)], []
+        )
+
+    def test_compute_angular_windspeeds_does_not_mutate_model_kwargs(self):
+        """The caller's ``model_kwargs`` dict must not be modified in place."""
+        si_track, d_centr, mask = self._er_2011_setup()
+        model_kwargs = {}
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            compute_angular_windspeeds(
+                si_track,
+                d_centr,
+                mask,
+                MODEL_VANG["ER11"],
+                cyclostrophic=True,
+                model_kwargs=model_kwargs,
+            )
+        self.assertEqual(model_kwargs, {})
+
     def test_vtrans_pass(self):
         """Test _vtrans function. Compare to MATLAB reference."""
         tc_track = TCTracks.from_processed_ibtracs_csv(TEST_TRACK)

climada/hazard/trop_cyclone/trop_cyclone_windfields.py

@@ -100,7 +100,7 @@ def compute_angular_windspeeds(
     d_centr: np.ndarray,
     mask_centr_close: np.ndarray,
     model: int,
-    cyclostrophic: Optional[bool] = False,
+    cyclostrophic: Optional[bool] = None,
     model_kwargs: Optional[dict] = None,
 ):
     """Compute (absolute) angular wind speeds according to a parametric wind profile
@@ -120,14 +120,16 @@ def compute_angular_windspeeds(
         If given, forward these kwargs to the selected model. Default: None
     cyclostrophic: bool, optional, deprecated
         This argument is deprecated and will be removed in a future release.
-        Include `cyclostrophic` as `model_kwargs` instead.
+        Include `cyclostrophic` as `model_kwargs` instead. If given, it takes precedence
+        over the value in `model_kwargs`. Default: None (not set).
 
     Returns
     -------
     ndarray of shape (npositions, ncentroids)
         containing the magnitude of the angular windspeed per track position per centroid location
     """
-    model_kwargs = {} if model_kwargs is None else model_kwargs
+    # copy, so that the deprecation shim below never mutates the caller's dict
+    model_kwargs = {} if model_kwargs is None else dict(model_kwargs)
 
     if cyclostrophic is not None:
         warnings.warn(