Fix protodisk Rmin/radius_min rename

Author: LourensVeen

src/amuse/ext/protodisk.py

@@ -1,5 +1,6 @@
 import numpy
 import warnings
+from amuse.support.helpers import rename_fn_par
 from amuse.ext.evrard_test import body_centered_grid_unit_cube
 from amuse.ext.evrard_test import regular_grid_unit_cube
 from amuse.ext.evrard_test import uniform_random_unit_cube
@@ -54,45 +55,15 @@ def make_xyz(self):
 class ProtoPlanetaryDisk:
     def __init__(
         self, targetN, convert_nbody=None, discfraction=0.1,
-        densitypower=1., thermalpower=0.5, radius_min=1, radius_max=100,
+        densitypower=1., thermalpower=0.5, radius_min=None, radius_max=None,
         gamma=1., q_out=2., base_grid=None, Rmin=None, Rmax=None,
     ):
-        if Rmin is not None:
-            warnings.warn(
-                "Rmin is deprecated, use radius_min instead",
-                category=FutureWarning,
-            )
-            if radius_min is not None and radius_min != Rmin:
-                raise ValueError(
-                    "Rmin and radius_min have different values, "
-                    "this is only allowed if one of them is None"
-                )
-            radius_min = Rmin
-
-        if radius_min is None:
-            raise ValueError("radius_min must be set")
-
-        if Rmax is not None:
-            warnings.warn(
-                "Rmax is deprecated, use radius_max instead",
-                category=FutureWarning,
-            )
-            if radius_max is not None and radius_max != Rmax:
-                raise ValueError(
-                    "Rmax and radius_max have different values, "
-                    "this is only allowed if one of them is None"
-                )
-            radius_max = Rmax
-
-        if radius_max is None:
-            raise ValueError("radius_max must be set")
-
         self.targetN = targetN
         self.convert_nbody = convert_nbody
         self.densitypower = densitypower
         self.thermalpower = thermalpower
-        self.Rmin = radius_min
-        self.Rmax = radius_max
+        self.Rmin = rename_fn_par("radius_min", radius_min, "Rmin", Rmin, 1)
+        self.Rmax = rename_fn_par("radius_max", radius_max, "Rmax", Rmax, 100)
         self.gamma = gamma
         self.q_out = q_out
         self.discfraction = discfraction
@@ -101,13 +72,22 @@ def __init__(
         self.a2 = self.thermalpower/2
         self.g = densitypower
         self.g2 = 2 - densitypower
-        self.k_out = ((1 + discfraction) / Rmax**3)**0.5
+        self.k_out = ((1 + discfraction) / self.Rmax**3)**0.5
         self.sigma_out = self.g2 * discfraction / (
-                2 * numpy.pi * Rmax**self.g * (Rmax**self.g2 - Rmin**self.g2))
+                2 * numpy.pi * self.Rmax**self.g *
+                (self.Rmax**self.g2 - self.Rmin**self.g2))
         self.cs_out = self.q_out * numpy.pi * self.sigma_out / self.k_out
 
         self.base_cylinder = uniform_unit_cylinder(targetN, base_grid)
 
+    @property
+    def radius_min(self):
+        return self.Rmin
+
+    @property
+    def radius_max(self):
+        return self.Rmax
+
     def sigma(self, r):
         return self.sigma_out * (self.Rmax / r)**self.g
 

src/amuse/support/helpers.py

@@ -0,0 +1,55 @@
+import warnings
+
+
+def rename_fn_par(new_name, new_value, old_name, old_value, default_value):
+    """Get value of a renamed function parameter.
+
+    If you have a function f with parameter ``x`` and default value 1, like this:
+
+    .. code-block::
+
+        def f(x=1):
+            ...
+
+    and you want to rename ``x`` to ``y`` while staying backwards compatible, then you
+    can rename ``x`` to ``y``, add ``x`` back in at the end so that old keyword
+    arguments still work, give both a default value of ``None``, and use this function
+    like this:
+
+    .. code-block::
+
+        def f(y=None, x=None):
+            value = rename_fn_par("y", y, "x", x, 1)
+
+    Any callers using ``x`` explicitly will receive a warning to change their code to
+    use ``y`` in the future. If both ``x`` and ``y`` are set and the values are
+    different, then an exception is raised.
+
+    Args:
+        new_name (str): The new name of the variable
+        new_value: The value passed using the new name
+        old_name (str): The old name of the variable
+        old_value: The value passed using the old name
+        default_value: The default value if neither are set
+
+    Returns:
+        Either new_value or old_value if only one is set or they're set to the same
+        value, or default_value if neither is set.
+
+    Raises:
+        ValueError: If both new_value and old_value are set, and to different values.
+    """
+    if new_value is not None:
+        if old_value is not None and old_value != new_value:
+            raise ValueError(
+                    f"{old_name} and {new_name} have different values,"
+                    " which is not allowed because they represent the same thing.")
+        return new_value
+
+    if old_value is not None:
+        warnings.warn(
+                f"{old_name} is deprecated, please use {new_name} instead",
+                category=FutureWarning)
+        return old_value
+
+    return default_value

src/tests/core_tests/test_helpers.py

@@ -0,0 +1,40 @@
+from amuse.support.helpers import rename_fn_par
+
+import pytest
+
+
+@pytest.mark.filterwarnings("ignore: old is deprecated.*")
+def test_rename_fn_par():
+    assert rename_fn_par("new", None, "old", None, 1) == 1
+    assert rename_fn_par("new", None, "old", 1, 2) == 1
+    assert rename_fn_par("new", 1, "old", None, 2) == 1
+    assert rename_fn_par("new", 1, "old", 1, 2) == 1
+
+    with pytest.raises(ValueError):
+        rename_fn_par("new", 1, "old", 2, 3)
+
+
+class _RenamedMethod:
+    def __init__(self, new = None, a = 1, b = "test", old = None):
+        self.y = rename_fn_par("new", new, "old", old, 42)
+
+
+@pytest.mark.filterwarnings("ignore: old is deprecated.*")
+def test_rename_fn_par_usage():
+    rm = _RenamedMethod()
+    assert rm.y == 42
+
+    rm = _RenamedMethod(1)
+    assert rm.y == 1
+
+    rm = _RenamedMethod(new=1)
+    assert rm.y == 1
+
+    rm = _RenamedMethod(old=1)
+    assert rm.y == 1
+
+    rm = _RenamedMethod(1, 1, "test", 1)
+    assert rm.y == 1
+
+    with pytest.raises(ValueError):
+        _RenamedMethod(1, 2, "test", 3)