fix: R wrapper cleanup, DataFrame mutation bug, test coverage and docs

DESCRIPTION

@@ -9,4 +9,7 @@ License: BSD 3-Clause
 Encoding: UTF-8
 LazyData: true
 Imports:
+    CircE (>= 1.1),
     sn
+Remotes:
+    CircE=MitchellAcoustics/CircE-R

src/soundscapy/__init__.py

@@ -1,6 +1,8 @@
 """Soundscapy is a Python library for soundscape analysis and visualisation."""
 
 # ruff: noqa: E402
+import importlib
+
 from loguru import logger
 
 # https://loguru.readthedocs.io/en/latest/resources/recipes.html#configuring-loguru-to-be-used-by-a-library-or-an-application
@@ -89,37 +91,68 @@
     # Audio module not available - this is expected if dependencies aren't installed
     pass
 
-# Try to import optional SPI module
-try:
-    from soundscapy import spi
-    from soundscapy.spi import (
-        CentredParams,
-        DirectParams,
-        MultiSkewNorm,
-        cp2dp,
-        dp2cp,
-        msn,
-    )
-
-    __all__ += [
+# Optional R-backed modules (spi, satp) are loaded lazily via __getattr__ so
+# that `import soundscapy` does not start the R process.  R only starts when
+# the user explicitly accesses one of these names.
+_SPI_ATTRS: frozenset[str] = frozenset(
+    {
+        "spi",
         "CentredParams",
         "DirectParams",
         "MultiSkewNorm",
         "cp2dp",
         "dp2cp",
         "msn",
-        "spi",
-    ]
-except ImportError:
-    # SPI module not available
-    pass
+        "spi_score",
+    }
+)
+_SATP_ATTRS: frozenset[str] = frozenset({"satp", "SATP", "CircModelE"})
 
-try:
-    from soundscapy import satp
-    from soundscapy.satp import SATP, CircModelE
 
-    __all__ += ["SATP", "CircModelE", "satp"]
+def __getattr__(name: str):  # noqa: ANN202
+    """
+    Lazily import optional R-backed sub-modules on first access.
 
-except ImportError:
-    # SATP module not available
-    pass
+    R is not started until one of these names is explicitly accessed.
+    After the first access each name is stored in the module's ``__dict__``,
+    so subsequent lookups skip this function entirely.
+    """
+    if name in _SPI_ATTRS:
+        try:
+            _spi = importlib.import_module("soundscapy.spi")
+            _g = globals()
+            _g["spi"] = _spi
+            # Pull the individual public names from the sub-module so callers
+            # can do ``sspy.MultiSkewNorm`` as well as ``sspy.spi.MultiSkewNorm``.
+            for _attr in _SPI_ATTRS - {"spi"}:
+                _g[_attr] = getattr(_spi, _attr)
+            return _g[name]
+        except ImportError as e:
+            msg = (
+                f"soundscapy.{name} requires optional SPI dependencies. "
+                "Install with: pip install 'soundscapy[spi]'"
+            )
+            raise ImportError(msg) from e
+
+    if name in _SATP_ATTRS:
+        try:
+            _satp = importlib.import_module("soundscapy.satp")
+            _g = globals()
+            _g["satp"] = _satp
+            for _attr in _SATP_ATTRS - {"satp"}:
+                _g[_attr] = getattr(_satp, _attr)
+            return _g[name]
+        except ImportError as e:
+            msg = (
+                f"soundscapy.{name} requires optional SATP dependencies. "
+                "Install with: pip install 'soundscapy[satp]'"
+            )
+            raise ImportError(msg) from e
+
+    msg = f"module 'soundscapy' has no attribute {name!r}"
+    raise AttributeError(msg)
+
+
+def __dir__() -> list[str]:
+    """Extend dir() to include lazily-loaded optional names (PEP 562)."""
+    return sorted(list(globals()) + list(_SPI_ATTRS) + list(_SATP_ATTRS))

src/soundscapy/r_wrapper/__init__.py

@@ -13,9 +13,9 @@
     raise ImportError(msg) from e
 
 # Now we can import our modules that depend on the optional packages
-from ._circe_wrapper import bfgs, extract_bfgs_fit
-from ._r_wrapper import PKG_SRC, get_r_session
-from ._rsn_wrapper import (
+from ._circe_wrapper import bfgs, extract_bfgs_fit  # noqa: F401
+from ._r_wrapper import PKG_SRC, get_r_session  # noqa: F401
+from ._rsn_wrapper import (  # noqa: F401
     cp2dp,
     dp2cp,
     extract_cp,
@@ -25,16 +25,7 @@
     selm,
 )
 
-__all__ = [
-    "PKG_SRC",
-    "bfgs",
-    "cp2dp",
-    "dp2cp",
-    "extract_bfgs_fit",
-    "extract_cp",
-    "extract_dp",
-    "get_r_session",
-    "sample_msn",
-    "sample_mtsn",
-    "selm",
-]
+# r_wrapper is an internal implementation package.  All user-facing names are
+# re-exported from soundscapy.spi and soundscapy.satp.  Nothing is in __all__
+# so that ``from soundscapy.r_wrapper import *`` imports nothing.
+__all__: list[str] = []

src/soundscapy/r_wrapper/_circe_wrapper.py

@@ -1,6 +1,8 @@
+import numpy as np
 import pandas as pd
 from rpy2 import robjects as ro
 from rpy2.robjects import pandas2ri
+from scipy.stats import chi2 as scipy_chi2
 
 from soundscapy.sspylogging import get_logger
 from soundscapy.surveys.survey_utils import PAQ_IDS
@@ -9,9 +11,6 @@
 
 logger = get_logger()
 
-_, _, _stats_package, _base_package, circe = get_r_session()
-logger.debug("R session and packages retrieved successfully.")
-
 
 def extract_bfgs_fit(bfgs_model: ro.ListVector) -> dict:
     """
@@ -33,10 +32,11 @@ def extract_bfgs_fit(bfgs_model: ro.ListVector) -> dict:
         >>> data_paqs = data[PAQ_IDS]
         >>> data_paqs = data_paqs.dropna()
         >>> data_cor = data_paqs.corr()
-        >>> n = data_paqs.shape[0]
+        >>> n = len(data_paqs)
         >>> circ_model = ModelType(name=CircModelE.CIRCUMPLEX)
         >>> circe_res = sspy.spi.bfgs(
         ... data_cor=data_cor,
+        ... n=n,
         ... scales=PAQ_IDS,
         ... m_val=3,
         ... equal_ang=circ_model.equal_ang,
@@ -45,18 +45,38 @@ def extract_bfgs_fit(bfgs_model: ro.ListVector) -> dict:
         >>> fit_stats = sspy.r_wrapper.extract_bfgs_fit(circe_res)
 
     """
+    # Session must already be active (bfgs_model was produced by bfgs()), but
+    # calling get_r_session() here ensures a clean error if somehow called in
+    # isolation.
+    get_r_session()
     with (ro.default_converter + pandas2ri.converter).context():
         py_res = {
             key.lower(): ro.conversion.get_conversion().rpy2py(val)
             for key, val in bfgs_model.items()
         }
-    py_res["p"] = 1 - _stats_package.pchisq(py_res["chisq"], py_res["dfnull"]).item()
+
+    # Normalize all length-1 numpy arrays to Python scalars so callers
+    # never need to call .item() themselves.  Vectors/matrices are kept
+    # as-is.  This also avoids DeprecationWarning from numpy >= 1.25 when
+    # float() or int() is applied to an ndarray with ndim > 0.
+    py_res = {
+        k: (v.item() if isinstance(v, np.ndarray) and v.shape == (1,) else v)
+        for k, v in py_res.items()
+    }
+
+    # Use scipy instead of R's pchisq to avoid py2rpy conversion of pandas
+    # Series objects produced by the pandas2ri context above.
+    # scipy.chi2.sf(x, df) == 1 - pchisq(x, df) by definition.
+    # Use the model's own degrees of freedom ("d"), NOT the null-model df
+    # ("dfnull" = k*(k-1)/2).  Using dfnull gives a wildly wrong p-value.
+    py_res["p"] = float(scipy_chi2.sf(py_res["chisq"], py_res["d"]))
 
     return py_res
 
 
 def bfgs(
     data_cor: pd.DataFrame,
+    n: int,
     scales: list[str] = PAQ_IDS,
     m_val: int = 3,
     *,
@@ -69,6 +89,8 @@ def bfgs(
     Parameters
     ----------
         data_cor (pd.DataFrame): Correlation matrix of the data.
+        n (int): Number of observations (participants) used to compute the correlation
+            matrix. Used by CircE_BFGS for chi-square and RMSEA calculations.
         scales (list[str], optional): List of scale names. Defaults to PAQ_IDS.
         m_val (int, optional): Number of dimensions. Defaults to 3.
         equal_ang (bool, optional): Whether to enforce equal angles constraint.
@@ -92,23 +114,26 @@ def bfgs(
         >>> circ_model = ModelType(name=CircModelE.CIRCUMPLEX)
         >>> circe_res = bfgs(
         ... data_cor=data_cor,
+        ... n=n,
         ... scales=PAQ_IDS,
         ... m_val=3,
         ... equal_ang=circ_model.equal_ang,
         ... equal_com=circ_model.equal_com,
         ... )
 
     """
-    n = data_cor.shape[0]
-
+    r = get_r_session()
     with (ro.default_converter + pandas2ri.converter).context():
+        # Only the Python→R conversion needs the pandas2ri context.
+        # Calling as_matrix() inside the context would cause its R-matrix
+        # return value to be auto-converted back to numpy by the active
+        # converter, producing a numpy array instead of an R matrix.
         r_data_cor = ro.conversion.get_conversion().py2rpy(data_cor)
 
-    r_cor_mat = _base_package.as_matrix(r_data_cor)
-
+    r_cor_mat = r.base.as_matrix(r_data_cor)
     r_scales = ro.StrVector(scales)
 
-    return circe.CircE_BFGS(
+    return r.circe.CircE_BFGS(
         r_cor_mat,
         v_names=r_scales,
         m=m_val,