test(fitfunctions): improve test quality and refactor combined_popt_psigma

.claude/commands/swp/test/audit.md

@@ -19,11 +19,12 @@ Detects anti-patterns BEFORE they cause test failures.
 
 | ID | Pattern | Severity | Count (baseline) |
 |----|---------|----------|------------------|
-| swp-test-001 | `assert X is not None` (trivial) | warning | 133 |
+| swp-test-001 | `assert X is not None` (trivial) | warning | 74 |
 | swp-test-002 | `patch.object` without `wraps=` | warning | 76 |
 | swp-test-003 | Assert without error message | info | - |
 | swp-test-004 | `plt.subplots()` (verify cleanup) | info | 59 |
 | swp-test-006 | `len(x) > 0` without type check | info | - |
+| swp-test-009 | `isinstance(X, object)` (disguised trivial) | warning | 0 |
 
 ### Good Patterns to Track (Adoption Metrics)
 
@@ -77,6 +78,15 @@ mcp__ast-grep__find_code(
     language="python",
     max_results=30
 )
+
+# 5. Disguised trivial assertion (swp-test-009)
+# isinstance(X, object) is equivalent to X is not None
+mcp__ast-grep__find_code(
+    project_folder="/path/to/SolarWindPy",
+    pattern="isinstance($OBJ, object)",
+    language="python",
+    max_results=50
+)
 ```
 
 **FALLBACK: CLI ast-grep (requires local `sg` installation)**
@@ -163,6 +173,7 @@ This skill is for **routine audits** - quick pattern detection before/during tes
 | Anti-Pattern | Fix | TEST_PATTERNS.md Section |
 |--------------|-----|-------------------------|
 | `assert X is not None` | `assert isinstance(X, Type)` | #6 Return Type Verification |
+| `isinstance(X, object)` | `isinstance(X, SpecificType)` | #6 Return Type Verification |
 | `patch.object(i, m)` | `patch.object(i, m, wraps=i.m)` | #1 Mock-with-Wraps |
 | Missing `plt.close()` | Add at test end | #15 Resource Cleanup |
 | Default parameter values | Use distinctive values (77, 2.5) | #2 Parameter Passthrough |

solarwindpy/fitfunctions/core.py

@@ -10,7 +10,9 @@
 import pdb  # noqa: F401
 import logging  # noqa: F401
 import warnings
+
 import numpy as np
+import pandas as pd
 
 from abc import ABC, abstractmethod
 from collections import namedtuple
@@ -336,23 +338,17 @@ def popt(self):
     def psigma(self):
         return dict(self._psigma)
 
-    @property
-    def psigma_relative(self):
-        return {k: v / self.popt[k] for k, v in self.psigma.items()}
-
     @property
     def combined_popt_psigma(self):
-        r"""Convenience to extract all versions of the optimized parameters."""
-        #         try:
-        popt = self.popt
-        psigma = self.psigma
-        prel = self.psigma_relative
-        #         except AttributeError:
-        #             popt = {k: np.nan for k in self.argnames}
-        #             psigma = {k: np.nan for k in self.argnames}
-        #             prel = {k: np.nan for k in self.argnames}
+        r"""Return optimized parameters and uncertainties as a DataFrame.
 
-        return {"popt": popt, "psigma": psigma, "psigma_relative": prel}
+        Returns
+        -------
+        pd.DataFrame
+            DataFrame with columns 'popt' and 'psigma', indexed by parameter names.
+            Relative uncertainty can be computed as: df['psigma'] / df['popt']
+        """
+        return pd.DataFrame({"popt": self.popt, "psigma": self.psigma})
 
     @property
     def pcov(self):

tests/fitfunctions/conftest.py

@@ -2,10 +2,23 @@
 
 from __future__ import annotations
 
+import matplotlib.pyplot as plt
 import numpy as np
 import pytest
 
 
+@pytest.fixture(autouse=True)
+def clean_matplotlib():
+    """Clean matplotlib state before and after each test.
+
+    Pattern sourced from tests/plotting/test_fixtures_utilities.py:37-43
+    which has been validated in production test runs.
+    """
+    plt.close("all")
+    yield
+    plt.close("all")
+
+
 @pytest.fixture
 def simple_linear_data():
     """Noisy linear data with unit weights.

tests/fitfunctions/test_core.py

@@ -1,14 +1,19 @@
 import numpy as np
+import pandas as pd
 import pytest
 from types import SimpleNamespace
 
+from scipy.optimize import OptimizeResult
+
 from solarwindpy.fitfunctions.core import (
     FitFunction,
     ChisqPerDegreeOfFreedom,
     InitialGuessInfo,
     InvalidParameterError,
     InsufficientDataError,
 )
+from solarwindpy.fitfunctions.plots import FFPlot
+from solarwindpy.fitfunctions.tex_info import TeXinfo
 
 
 def linear_function(x, m, b):
@@ -144,12 +149,12 @@ def test_make_fit_success_failure(monkeypatch, simple_linear_data, small_n):
     x, y, w = simple_linear_data
     lf = LinearFit(x, y, weights=w)
     lf.make_fit()
-    assert isinstance(lf.fit_result, object)
+    assert isinstance(lf.fit_result, OptimizeResult)
     assert set(lf.popt) == {"m", "b"}
     assert set(lf.psigma) == {"m", "b"}
     assert lf.pcov.shape == (2, 2)
     assert isinstance(lf.chisq_dof, ChisqPerDegreeOfFreedom)
-    assert lf.plotter is not None and lf.TeX_info is not None
+    assert isinstance(lf.plotter, FFPlot) and isinstance(lf.TeX_info, TeXinfo)
 
     x, y, w = small_n
     lf_small = LinearFit(x, y, weights=w)
@@ -187,19 +192,24 @@ def test_str_call_and_properties(fitted_linear):
     assert isinstance(lf.fit_bounds, dict)
     assert isinstance(lf.chisq_dof, ChisqPerDegreeOfFreedom)
     assert lf.dof == lf.observations.used.y.size - len(lf.p0)
-    assert lf.fit_result is not None
+    assert isinstance(lf.fit_result, OptimizeResult)
     assert isinstance(lf.initial_guess_info["m"], InitialGuessInfo)
     assert lf.nobs == lf.observations.used.x.size
-    assert lf.plotter is not None
+    assert isinstance(lf.plotter, FFPlot)
     assert set(lf.popt) == {"m", "b"}
     assert set(lf.psigma) == {"m", "b"}
-    assert set(lf.psigma_relative) == {"m", "b"}
+    # combined_popt_psigma returns DataFrame; psigma_relative is trivially computable
     combined = lf.combined_popt_psigma
-    assert set(combined) == {"popt", "psigma", "psigma_relative"}
+    assert isinstance(combined, pd.DataFrame)
+    assert set(combined.columns) == {"popt", "psigma"}
+    assert set(combined.index) == {"m", "b"}
+    # Verify relative uncertainty is trivially computable from DataFrame
+    psigma_relative = combined["psigma"] / combined["popt"]
+    assert set(psigma_relative.index) == {"m", "b"}
     assert lf.pcov.shape == (2, 2)
     assert 0.0 <= lf.rsq <= 1.0
     assert lf.sufficient_data is True
-    assert lf.TeX_info is not None
+    assert isinstance(lf.TeX_info, TeXinfo)
 
 
 # ============================================================================
@@ -265,7 +275,7 @@ def fake_ls(func, p0, **kwargs):
 
         bounds_dict = {"m": (-10, 10), "b": (-5, 5)}
         res, p0 = lf._run_least_squares(bounds=bounds_dict)
-        assert captured["bounds"] is not None
+        assert isinstance(captured["bounds"], (list, tuple, np.ndarray))
 
 
 class TestCallableJacobian:

tests/fitfunctions/test_exponentials.py

@@ -9,7 +9,9 @@
     ExponentialPlusC,
     ExponentialCDF,
 )
-from solarwindpy.fitfunctions.core import InsufficientDataError
+from scipy.optimize import OptimizeResult
+
+from solarwindpy.fitfunctions.core import ChisqPerDegreeOfFreedom, InsufficientDataError
 
 
 @pytest.mark.parametrize(
@@ -132,11 +134,11 @@ def test_make_fit_success_regular(exponential_data):
         # Test fitting succeeds
         obj.make_fit()
 
-        # Test fit results are available
-        assert obj.popt is not None
-        assert obj.pcov is not None
-        assert obj.chisq_dof is not None
-        assert obj.fit_result is not None
+        # Test fit results are available with correct types
+        assert isinstance(obj.popt, dict)
+        assert isinstance(obj.pcov, np.ndarray)
+        assert isinstance(obj.chisq_dof, ChisqPerDegreeOfFreedom)
+        assert isinstance(obj.fit_result, OptimizeResult)
 
         # Test output shapes
         assert len(obj.popt) == len(obj.p0)
@@ -154,11 +156,11 @@ def test_make_fit_success_cdf(exponential_data):
     # Test fitting succeeds
     obj.make_fit()
 
-    # Test fit results are available
-    assert obj.popt is not None
-    assert obj.pcov is not None
-    assert obj.chisq_dof is not None
-    assert obj.fit_result is not None
+    # Test fit results are available with correct types
+    assert isinstance(obj.popt, dict)
+    assert isinstance(obj.pcov, np.ndarray)
+    assert isinstance(obj.chisq_dof, ChisqPerDegreeOfFreedom)
+    assert isinstance(obj.fit_result, OptimizeResult)
 
     # Test output shapes
     assert len(obj.popt) == len(obj.p0)
@@ -303,8 +305,8 @@ def test_property_access_before_fit(cls):
     obj = cls(x, y)
 
     # These should work before fitting
-    assert obj.TeX_function is not None
-    assert obj.p0 is not None
+    assert isinstance(obj.TeX_function, str)
+    assert isinstance(obj.p0, list)
 
     # These should raise AttributeError before fitting
     with pytest.raises(AttributeError):
@@ -324,7 +326,7 @@ def test_exponential_with_weights(exponential_data):
     obj.make_fit()
 
     # Should complete successfully
-    assert obj.popt is not None
+    assert isinstance(obj.popt, dict)
     assert len(obj.popt) == 2
 
 

tests/fitfunctions/test_lines.py

@@ -8,7 +8,7 @@
     Line,
     LineXintercept,
 )
-from solarwindpy.fitfunctions.core import InsufficientDataError
+from solarwindpy.fitfunctions.core import ChisqPerDegreeOfFreedom, InsufficientDataError
 
 
 @pytest.mark.parametrize(
@@ -103,10 +103,10 @@ def test_make_fit_success(cls, simple_linear_data):
     # Test fitting succeeds
     obj.make_fit()
 
-    # Test fit results are available
-    assert obj.popt is not None
-    assert obj.pcov is not None
-    assert obj.chisq_dof is not None
+    # Test fit results are available with correct types
+    assert isinstance(obj.popt, dict)
+    assert isinstance(obj.pcov, np.ndarray)
+    assert isinstance(obj.chisq_dof, ChisqPerDegreeOfFreedom)
 
     # Test output shapes
     assert len(obj.popt) == len(obj.p0)
@@ -231,7 +231,7 @@ def test_line_with_weights(simple_linear_data):
     obj.make_fit()
 
     # Should complete successfully
-    assert obj.popt is not None
+    assert isinstance(obj.popt, dict)
     assert len(obj.popt) == 2
 
 
@@ -290,8 +290,8 @@ def test_property_access_before_fit(cls):
     obj = cls(x, y)
 
     # These should work before fitting
-    assert obj.TeX_function is not None
-    assert obj.p0 is not None
+    assert isinstance(obj.TeX_function, str)
+    assert isinstance(obj.p0, list)
 
     # These should raise AttributeError before fitting
     with pytest.raises(AttributeError):

tests/fitfunctions/test_metaclass_compatibility.py

@@ -36,7 +36,7 @@ class TestMeta(FitFunctionMeta):
                 pass
 
             # Metaclass should have valid MRO
-            assert TestMeta.__mro__ is not None
+            assert isinstance(TestMeta.__mro__, tuple)
         except TypeError as e:
             if "consistent method resolution" in str(e).lower():
                 pytest.fail(f"MRO conflict detected: {e}")
@@ -79,7 +79,7 @@ def TeX_function(self):
         # Should instantiate successfully
         x, y = [0, 1, 2], [0, 1, 2]
         fit_func = CompleteFitFunction(x, y)
-        assert fit_func is not None
+        assert isinstance(fit_func, FitFunction)
         assert hasattr(fit_func, "function")
 
 
@@ -110,7 +110,7 @@ class ChildFit(ParentFit):
             pass
 
         # Docstring should exist (inheritance working)
-        assert ChildFit.__doc__ is not None
+        assert isinstance(ChildFit.__doc__, str)
         assert len(ChildFit.__doc__) > 0
 
     def test_inherited_method_docstrings(self):
@@ -139,12 +139,13 @@ def test_import_all_fitfunctions(self):
             TrendFit,
         )
 
-        # All imports successful
-        assert Exponential is not None
-        assert Gaussian is not None
-        assert PowerLaw is not None
-        assert Line is not None
-        assert Moyal is not None
+        # All imports successful - verify they are proper FitFunction subclasses
+        assert issubclass(Exponential, FitFunction)
+        assert issubclass(Gaussian, FitFunction)
+        assert issubclass(PowerLaw, FitFunction)
+        assert issubclass(Line, FitFunction)
+        assert issubclass(Moyal, FitFunction)
+        # TrendFit is not a FitFunction subclass, just verify it exists
         assert TrendFit is not None
 
     def test_instantiate_all_fitfunctions(self):
@@ -166,7 +167,9 @@ def test_instantiate_all_fitfunctions(self):
         for FitClass in fitfunctions:
             try:
                 instance = FitClass(x, y)
-                assert instance is not None, f"{FitClass.__name__} instantiation failed"
+                assert isinstance(
+                    instance, FitFunction
+                ), f"{FitClass.__name__} instantiation failed"
                 assert hasattr(
                     instance, "function"
                 ), f"{FitClass.__name__} missing function property"