Merge pull request #1074 from RocketPy-Team/bug/pre-release-v1.13.0-review-fixes

BUG/MNT: pre-release v1.13.0 review fixes

Author: Gui-FernandesBR

CHANGELOG.md

@@ -32,7 +32,6 @@ Attention: The newest changes should be on top -->
 
 ### Added
 
-- ENH: MNT: pre-release v1.13.0 cleanup — changelog consolidation + optional deps sync [#1073](https://github.com/RocketPy-Team/RocketPy/pull/1073)
 ### Changed
 
 ### Fixed
@@ -72,8 +71,10 @@ Attention: The newest changes should be on top -->
 - MNT: Remove unused pylint disable statements [#1067](https://github.com/RocketPy-Team/RocketPy/pull/1067)
 - MNT: Discrete controllers are now called exactly once per time node (previously twice); results may change for stateful controllers and `observed_variables` no longer contains duplicated entries [#949](https://github.com/RocketPy-Team/RocketPy/pull/949)
 - MNT: Multi-dimensional linear `Function` objects now apply their extrapolation rule to points outside the data's convex hull (previously returned NaN) [#969](https://github.com/RocketPy-Team/RocketPy/pull/969)
-- MNT: Informational messages previously shown with `print()` now use Python logging and are silent by default; call `rocketpy.utils.enable_logging()` to see them [#973](https://github.com/RocketPy-Team/RocketPy/pull/973)
+- MNT: Informational messages previously shown with `print()` now use Python logging and are silent by default; call `rocketpy.utilities.enable_logging()` to see them [#973](https://github.com/RocketPy-Team/RocketPy/pull/973)
 - ENH: Refactor flight.py latitude/longitude to use inverted_haversine [#1055](https://github.com/RocketPy-Team/RocketPy/pull/1055)
+- MNT: `Function` with `interpolation="akima"` now matches the canonical (SciPy) Akima spline; interpolated values differ slightly from previous releases [#1049](https://github.com/RocketPy-Team/RocketPy/pull/1049)
+- MNT: `Function.differentiate` on 1-D array Functions now returns the analytical derivative of the interpolation (previously central finite differences); values at knots and domain edges may change [#1049](https://github.com/RocketPy-Team/RocketPy/pull/1049)
 
 ### Deprecated
 

docs/examples/index.rst

@@ -12,6 +12,13 @@ apogee of some rockets.
    :hide-code:
 
    import plotly.graph_objects as go
+   import plotly.io as pio
+
+   # Emit an HTML output that pulls plotly.js from the CDN, so the figure
+   # renders on the static documentation pages. Without an explicit renderer,
+   # ``fig.show()`` under jupyter-execute produces a plotly-mimetype bundle
+   # that a static HTML page has no frontend to display (blank chart).
+   pio.renderers.default = "notebook_connected"
 
    results = {
       # "Name (Year)": (simulated, measured) m

pyproject.toml

@@ -73,6 +73,7 @@ monte-carlo = [
 
 animation = [
     "pyvista>=0.45",
+    "imageio",
     "imageio-ffmpeg>=0.5"
 ]
 

rocketpy/environment/environment.py

@@ -38,6 +38,7 @@
     get_interval_date_from_time_array,
     get_pressure_levels_from_file,
     mask_and_clean_dataset,
+    pressure_unit_to_factor,
 )
 from rocketpy.environment.weather_model_mapping import WeatherModelMapping
 from rocketpy.mathutils.function import NUMERICAL_TYPES, Function, funcify_method
@@ -1158,9 +1159,7 @@ def __determine_pressure_conversion_factor(
         if pressure_conversion_factor is not None:
             # User explicitly supplied a value — honour it.
             if isinstance(pressure_conversion_factor, str):
-                return (
-                    100 if pressure_conversion_factor.lower() in ("mbar", "hpa") else 1
-                )
+                return pressure_unit_to_factor(pressure_conversion_factor)
             return pressure_conversion_factor
 
         # Auto-detect. Primary source: known-model lookup table.
@@ -1173,9 +1172,9 @@ def __determine_pressure_conversion_factor(
         _pa_files = {"GFS", "NAM", "RAP", "HRRR", "AIGFS"}
         if input_dict in _hpa_dicts or input_file in _hpa_dicts:
             return 100
-        if input_file in _hpa_files:
+        if input_dict in _hpa_files or input_file in _hpa_files:
             return 100
-        if input_file in _pa_files:
+        if input_dict in _pa_files or input_file in _pa_files:
             return 1
         return None
 
@@ -1360,11 +1359,7 @@ def set_atmospheric_model(  # pylint: disable=too-many-statements
                                 "Argument 'pressure_conversion_factor' must be strictly positive!"
                             )
                     if isinstance(pressure_conversion_factor, str):
-                        if pressure_conversion_factor.lower() not in (
-                            "mbar",
-                            "hpa",
-                            "pa",
-                        ):
+                        if pressure_unit_to_factor(pressure_conversion_factor) is None:
                             raise ValueError(
                                 "Argument 'pressure_conversion_factor' unit must be a standard pressure unit ('mbar', 'hPa', 'Pa')!"
                             )
@@ -2961,6 +2956,7 @@ def to_dict(self, **kwargs):
             "wind_direction_ensemble": getattr(self, "wind_direction_ensemble", None),
             "wind_speed_ensemble": getattr(self, "wind_speed_ensemble", None),
             "num_ensemble_members": getattr(self, "num_ensemble_members", None),
+            "ensemble_member": getattr(self, "ensemble_member", None),
         }
 
         if kwargs.get("include_outputs", False):
@@ -3027,6 +3023,7 @@ def from_dict(cls, data):  # pylint: disable=too-many-statements
             env.wind_direction_ensemble = data["wind_direction_ensemble"]
             env.wind_speed_ensemble = data["wind_speed_ensemble"]
             env.num_ensemble_members = data["num_ensemble_members"]
+            env.ensemble_member = data.get("ensemble_member", 0) or 0
 
         env.__reset_barometric_height_function()
         env.calculate_density_profile()

rocketpy/environment/tools.py

@@ -190,6 +190,35 @@ def geodesic_to_lambert_conformal(lat, lon, projection_variable, x_units="m"):
 ## These functions are meant to be used with netcdf4 datasets
 
 
+HPA_UNIT_SYNONYMS = frozenset(
+    {"hpa", "mbar", "mb", "millibar", "millibars", "hectopascal", "hectopascals"}
+)
+PA_UNIT_SYNONYMS = frozenset({"pa", "pascal"})
+
+
+def pressure_unit_to_factor(unit):
+    """Return the Pa conversion factor for a pressure-unit string.
+
+    Parameters
+    ----------
+    unit : str
+        Pressure unit label (case-insensitive), e.g. ``"hPa"``, ``"mb"``,
+        ``"millibar"`` or ``"Pa"``.
+
+    Returns
+    -------
+    int or None
+        ``100`` for hPa/millibar synonyms, ``1`` for Pa synonyms, or ``None``
+        if the unit string is not recognised.
+    """
+    unit = unit.lower().strip()
+    if unit in HPA_UNIT_SYNONYMS:
+        return 100
+    if unit in PA_UNIT_SYNONYMS:
+        return 1
+    return None
+
+
 def get_pressure_levels_from_file(data, dictionary, conversion_factor):
     """Extracts pressure levels from a netCDF4 dataset and converts them to Pa.
 
@@ -219,7 +248,7 @@ def get_pressure_levels_from_file(data, dictionary, conversion_factor):
         level_var = data.variables[dictionary["level"]]
         if conversion_factor is None:
             raw_units = getattr(level_var, "units", "").lower().strip()
-            if raw_units in ("hpa", "mbar", "millibars", "hectopascal", "hectopascals"):
+            if raw_units in HPA_UNIT_SYNONYMS:
                 conversion_factor = 100
             else:
                 conversion_factor = 1

rocketpy/mathutils/_calc/_fitting.py

@@ -100,6 +100,12 @@ def fit_akima(x, y):
     h = np.diff(x)
     m = np.diff(y) / h  # slopes of segments, shape (n-1,)
 
+    if n == 2:
+        # A single segment: Akima reduces to linear interpolation. The ghost-
+        # value reflection below indexes ``m[1]``/``m[-2]``, which do not exist
+        # for two points, so this case must be handled explicitly.
+        return np.vstack([y[:-1], m, np.zeros(1), np.zeros(1)])
+
     # Extend m with 2 ghost values on each side (Akima boundary reflection)
     # m_ext indices: 0..n+2  (n-1 interior + 2 left + 2 right)
     m_ext = np.empty(n + 3)
@@ -176,12 +182,15 @@ def fit_pchip(x, y):
         sign_change = (m[:-1] * m[1:]) <= 0
         pos_mask = ~sign_change
 
-        # Safe harmonic mean
-        t[1:-1] = np.where(
-            pos_mask,
-            (w1 + w2) / (w1 / m[:-1] + w2 / m[1:]),
-            0.0,
-        )
+        # Safe harmonic mean. The reciprocals are evaluated eagerly before
+        # ``np.where`` masks them, so silence the harmless divide-by-zero that
+        # occurs on flat segments (where ``pos_mask`` is already False).
+        with np.errstate(divide="ignore", invalid="ignore"):
+            t[1:-1] = np.where(
+                pos_mask,
+                (w1 + w2) / (w1 / m[:-1] + w2 / m[1:]),
+                0.0,
+            )
 
     # Boundary slopes: one-sided, clamped for monotonicity
     t[0] = _pchip_edge_slope(

rocketpy/motors/ring_cluster_motor.py

@@ -289,7 +289,9 @@ def info(self, *args, **kwargs):
         print("Cluster Configuration:")
         print(f" - Motors: {self.number} x {type(self.motor).__name__}")
         print(f" - Radial Distance: {self.radius} m")
-        return self.motor.info(*args, **kwargs)
+        # Report the cluster's own (N-times scaled) quantities, not the single
+        # base motor's, mirroring the correct behaviour of ``all_info``.
+        return super().info(*args, **kwargs)
 
     def to_dict(self, **kwargs):
         data = super().to_dict(**kwargs)

rocketpy/plots/flight_plots.py

@@ -888,10 +888,7 @@ def _animation_options(  # pylint: disable=too-many-statements
         options = {
             "background_color": kwargs.pop("background_color", None),
             "playback_controls": kwargs.pop("playback_controls", True),
-            "show_subrocket_point": kwargs.pop(
-                "show_subrocket_point",
-                kwargs.pop("show_subrocket_point", True),
-            ),
+            "show_subrocket_point": kwargs.pop("show_subrocket_point", True),
             "ground_image": kwargs.pop("ground_image", None),
             "ground_image_bounds": kwargs.pop("ground_image_bounds", None),
             "ground_image_coordinates": kwargs.pop("ground_image_coordinates", "enu"),

rocketpy/rocket/parachute.py

@@ -1,4 +1,4 @@
-from inspect import signature
+from inspect import Parameter, signature
 
 import numpy as np
 
@@ -302,6 +302,10 @@ def __evaluate_trigger_function(self, trigger):  # pylint: disable=too-many-stat
         def _make_wrapper(fn):
             sig = signature(fn)
             params = list(sig.parameters.keys())
+            has_var_positional = any(
+                param.kind is Parameter.VAR_POSITIONAL
+                for param in sig.parameters.values()
+            )
 
             # detect if user function expects acceleration-like argument
             expects_udot = any(
@@ -324,6 +328,12 @@ def wrapper(p, h, y, sensors, u_dot):
                 if num_params >= 5:
                     # Pass both sensors and u_dot
                     return fn(p, h, y, sensors, u_dot)
+                if has_var_positional:
+                    # Variadic triggers (e.g. ``lambda *args: ...``) accept any
+                    # number of positional arguments. Preserve the pre-1.13
+                    # behaviour of calling them with (pressure, height, state,
+                    # sensors).
+                    return fn(p, h, y, sensors)
                 # If function signature is not supported, raise an error
                 raise TypeError(
                     f"Trigger function '{fn.__name__}' has unsupported signature: "

tests/unit/environment/test_environment.py

@@ -14,6 +14,8 @@
     geodesic_to_utm,
     get_final_date_from_time_array,
     get_initial_date_from_time_array,
+    get_pressure_levels_from_file,
+    pressure_unit_to_factor,
     utm_to_geodesic,
 )
 from rocketpy.environment.weather_model_mapping import WeatherModelMapping
@@ -378,14 +380,21 @@ def test_environment_to_dict_from_dict_round_trip_preserves_weather_metadata(
         ensemble_metadata.update(
             {
                 "level_ensemble": np.array([1000.0, 900.0]),
-                "height_ensemble": np.array([[0.0, 1000.0]]),
-                "temperature_ensemble": np.array([[288.15, 281.15]]),
-                "wind_u_ensemble": np.array([[2.0, 3.0]]),
-                "wind_v_ensemble": np.array([[4.0, 5.0]]),
-                "wind_heading_ensemble": np.array([[26.565051, 30.963757]]),
-                "wind_direction_ensemble": np.array([[206.565051, 210.963757]]),
-                "wind_speed_ensemble": np.array([[4.472136, 5.830952]]),
-                "num_ensemble_members": 1,
+                "height_ensemble": np.array([[0.0, 1000.0], [0.0, 1000.0]]),
+                "temperature_ensemble": np.array([[288.15, 281.15], [288.15, 281.15]]),
+                "wind_u_ensemble": np.array([[2.0, 3.0], [2.0, 3.0]]),
+                "wind_v_ensemble": np.array([[4.0, 5.0], [4.0, 5.0]]),
+                "wind_heading_ensemble": np.array(
+                    [[26.565051, 30.963757], [26.565051, 30.963757]]
+                ),
+                "wind_direction_ensemble": np.array(
+                    [[206.565051, 210.963757], [206.565051, 210.963757]]
+                ),
+                "wind_speed_ensemble": np.array(
+                    [[4.472136, 5.830952], [4.472136, 5.830952]]
+                ),
+                "num_ensemble_members": 2,
+                "ensemble_member": 1,
             }
         )
 
@@ -416,6 +425,7 @@ def test_environment_to_dict_from_dict_round_trip_preserves_weather_metadata(
         npt.assert_allclose(restored_env.level_ensemble, env.level_ensemble)
         npt.assert_allclose(restored_env.height_ensemble, env.height_ensemble)
         assert restored_env.num_ensemble_members == env.num_ensemble_members
+        assert restored_env.ensemble_member == env.ensemble_member == 1
 
 
 class _DummyDataset:
@@ -818,3 +828,91 @@ def test_pressure_conversion_factor_autodetect_by_model(
         None, None, model
     )
     assert factor == expected_factor
+
+
+@pytest.mark.parametrize(
+    "model, expected_factor",
+    [("GEFS", 100), ("HIRESW", 100), ("GFS", 1), ("AIGFS", 1)],
+)
+def test_pressure_conversion_factor_autodetect_by_dictionary(
+    example_plain_env, model, expected_factor
+):
+    """Model shortcuts arriving via ``dictionary`` (the realistic download
+    path) must map to the same factor as when they arrive via ``file``."""
+    factor = example_plain_env._Environment__determine_pressure_conversion_factor(
+        None, model, None
+    )
+    assert factor == expected_factor
+
+
+@pytest.mark.parametrize(
+    "units, expected_levels",
+    [
+        ("mb", [100000.0, 85000.0]),
+        ("millibar", [100000.0, 85000.0]),
+        ("millibars", [100000.0, 85000.0]),
+        ("hPa", [100000.0, 85000.0]),
+        ("mbar", [100000.0, 85000.0]),
+        ("Pa", [1000.0, 850.0]),
+    ],
+)
+def test_get_pressure_levels_from_file_unit_synonyms(units, expected_levels):
+    """hPa/millibar unit synonyms auto-scale by 100; Pa by 1."""
+
+    class _Var:
+        def __init__(self, values, units):
+            self._values = np.asarray(values)
+            self.units = units
+
+        def __getitem__(self, key):
+            return self._values[key]
+
+    class _DS:
+        def __init__(self, var):
+            self.variables = {"lev": var}
+
+    dataset = _DS(_Var([1000.0, 850.0], units))
+    levels = get_pressure_levels_from_file(dataset, {"level": "lev"}, None)
+    npt.assert_allclose(levels, expected_levels)
+
+
+@pytest.mark.parametrize(
+    "unit, expected",
+    [
+        ("mbar", 100),
+        ("mb", 100),
+        ("hPa", 100),
+        ("millibar", 100),
+        ("millibars", 100),
+        ("hectopascal", 100),
+        ("Pa", 1),
+        ("pascal", 1),
+        ("parsecs", None),
+        ("", None),
+    ],
+)
+def test_pressure_unit_to_factor(unit, expected):
+    """The shared unit->factor helper: hPa synonyms ->100, Pa ->1, else None."""
+
+    assert pressure_unit_to_factor(unit) == expected
+
+
+@pytest.mark.parametrize("unit, expected", [("mb", 100), ("millibar", 100), ("Pa", 1)])
+def test_pressure_conversion_factor_explicit_unit_synonyms(
+    example_plain_env, unit, expected
+):
+    """An explicit string ``pressure_conversion_factor`` accepts the same unit
+    synonyms as file auto-detection (Copilot review consistency fix)."""
+    factor = example_plain_env._Environment__determine_pressure_conversion_factor(
+        unit, None, None
+    )
+    assert factor == expected
+
+
+def test_set_atmospheric_model_rejects_unknown_pressure_unit(example_plain_env):
+    """An unrecognized ``pressure_conversion_factor`` unit is rejected during
+    validation, before any file access."""
+    with pytest.raises(ValueError, match="pressure_conversion_factor"):
+        example_plain_env.set_atmospheric_model(
+            type="Forecast", file="dummy", pressure_conversion_factor="parsecs"
+        )