diff --git a/.claude/hooks/plan-completion-manager.py b/.claude/hooks/plan-completion-manager.py index b1b0e0a1..1d2aaeb9 100755 --- a/.claude/hooks/plan-completion-manager.py +++ b/.claude/hooks/plan-completion-manager.py @@ -256,7 +256,6 @@ def scan_and_archive_completed_plans(): # Scan all plan directories (excluding completed, abandoned, and special files) for item in plans_dir.iterdir(): if item.is_dir() and item.name not in ["completed", "abandoned"]: - print(f"šŸ“‹ Checking plan: {item.name}") if is_plan_completed(item): @@ -266,7 +265,7 @@ def scan_and_archive_completed_plans(): closeout_success = generate_closeout_documentation(item.name, item) if not closeout_success: print( - f"āš ļø Proceeding with archival despite closeout generation issues" + "āš ļø Proceeding with archival despite closeout generation issues" ) # Preserve branches before moving @@ -282,12 +281,12 @@ def scan_and_archive_completed_plans(): # Summary if moved_plans: - print(f"\nšŸ“Š Summary:") + print("\nšŸ“Š Summary:") print(f" Moved {len(moved_plans)} completed plans to plans/completed/") for plan in moved_plans: print(f" - {plan}") - print(f"\nšŸ”’ Branch Preservation:") + print("\nšŸ”’ Branch Preservation:") for branch_info in preserved_branches: if "error" not in branch_info: plan_name = branch_info["plan_name"] diff --git a/.claude/hooks/plan-scope-auditor.py b/.claude/hooks/plan-scope-auditor.py index 5f6a1429..6da2716e 100644 --- a/.claude/hooks/plan-scope-auditor.py +++ b/.claude/hooks/plan-scope-auditor.py @@ -316,8 +316,6 @@ def _assess_module_relevance(self, affects: str) -> float: def _assess_scientific_keywords(self, plan_text: str) -> float: """Assess scientific relevance based on keywords.""" - scores = [] - # High value keywords (physics/research terms) high_matches = sum( 1 @@ -522,7 +520,9 @@ def _analyze_module_impact(self, plan_data: Dict) -> str: impact_level = ( "High" if info["weight"] >= 0.8 - else "Medium" if info["weight"] >= 0.5 else "Low" + else "Medium" + if info["weight"] >= 0.5 + else "Low" ) analysis.append( f"- **{module}** ({impact_level} Impact): {info['description']}" diff --git a/docs/requirements.txt b/docs/requirements.txt index 9ce911de..5ff832d8 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,20 +1,9 @@ -# RTD-specific documentation requirements -# Enhanced with version pinning for RTD compatibility +# Documentation requirements generated from requirements-dev.txt +# DO NOT EDIT MANUALLY - regenerate with scripts/generate_docs_requirements.py -# Core documentation dependencies -sphinx>=6.0,<8.0 -sphinx_rtd_theme>=1.0.0 -numpydoc>=1.5.0 -sphinxcontrib-bibtex>=2.5.0 -docstring-inheritance>=2.0.0 - -# Quality and spell checking doc8 +numpydoc +sphinx +sphinx_rtd_theme sphinxcontrib-spelling - -# Scientific documentation support -matplotlib>=3.5.0 -numpy>=1.20.0 -pandas>=1.3.0 -scipy>=1.7.0 -astropy>=5.0.0 +sphinxcontrib-bibtex diff --git a/docs/source/fitfunctions_architecture.md b/docs/source/fitfunctions_architecture.md new file mode 100644 index 00000000..afd154ac --- /dev/null +++ b/docs/source/fitfunctions_architecture.md @@ -0,0 +1,206 @@ +# FitFunctions Architecture Design Document + +## Executive Summary + +This document analyzes the architectural design patterns implemented during the SolarWindPy fitfunctions submodule audit (Phases 1-2) and provides recommendations for Phase 3 improvements. The key achievement was implementing docstring inheritance through a metaclass approach, reducing documentation duplication by 83% while maintaining full NumPy-style documentation. + +## Current Architecture Overview + +### Core Design Pattern: Abstract Base Class with Metaclass Enhancement + +The fitfunctions module follows a classic **Template Method Pattern** enhanced with **Metaclass-based Documentation Inheritance**: + +```python +# Core metaclass combining ABC and docstring inheritance +class FitFunctionMeta(NumpyDocstringInheritanceMeta, type(ABC)): + """Metaclass combining ABC and docstring inheritance.""" + pass + +class FitFunction(ABC, metaclass=FitFunctionMeta): + # Template methods and comprehensive documentation +``` + +### Architecture Components + +#### 1. **Metaclass Architecture** (`FitFunctionMeta`) +- **Purpose**: Combines Abstract Base Class functionality with automatic docstring inheritance +- **Implementation**: Multiple inheritance from `NumpyDocstringInheritanceMeta` and `type(ABC)` +- **Benefits**: + - Enforces abstract method implementation + - Automatically inherits comprehensive documentation + - Reduces code duplication by 83% (440 lines → 73 lines) + +#### 2. **Abstract Base Class** (`FitFunction`) +- **Pattern**: Template Method Pattern +- **Core Template**: `make_fit()` orchestrates the fitting workflow +- **Abstract Properties**: `function`, `p0`, `TeX_function` +- **Concrete Methods**: Parameter management, plotting, statistics + +#### 3. **Subclass Implementations** (11 classes) +- **Gaussians**: `Gaussian`, `GaussianNormalized` +- **Exponentials**: `Exponential`, `ExponentialPlusC`, `ExponentialPlusCSin` +- **Power Laws**: `PowerLaw`, `PowerLawPlusC` +- **Lines**: `Line`, `Parabola` +- **Specialized**: `Moyal`, `MaxwellBoltzmann` + +### Key Architectural Decisions + +#### Decision 1: Metaclass over Composition +**Rationale**: Chose metaclass inheritance over composition patterns +- **Pros**: Automatic inheritance, no boilerplate, transparent to subclasses +- **Cons**: Complex debugging, metaclass interactions +- **Alternative Considered**: Decorator-based documentation injection +- **Outcome**: Successful - reduced duplication without changing subclass APIs + +#### Decision 2: Standardized Constructor Signature +**Rationale**: Unified `(xobs, yobs, **kwargs)` across all subclasses +- **Previous Issue**: `Moyal(sigma, xobs, yobs)` broke convention +- **Solution**: Standardized to parent signature, moved parameters to kwargs +- **Impact**: Fixed 12 test failures, improved API consistency + +#### Decision 3: Abstract Properties over Abstract Methods +**Rationale**: Used `@abstractproperty` for `function`, `p0`, `TeX_function` +- **Benefit**: Lazy evaluation, caching support, cleaner subclass API +- **Trade-off**: Python 3.3+ deprecation warnings (replaced with `@property + @abstractmethod`) + +## Architecture Strengths + +### 1. **Code Reuse and DRY Principle** +- 83% reduction in documentation duplication +- Comprehensive parameter documentation inherited by all subclasses +- Single source of truth for fitting workflow + +### 2. **Extensibility** +- Clear interface for new fit functions +- Only requires implementing 3 abstract properties +- Automatic integration with plotting and LaTeX generation + +### 3. **Consistency** +- Uniform constructor signatures +- Standardized parameter naming conventions +- Consistent error handling and logging + +### 4. **Scientific Computing Best Practices** +- Robust least squares fitting with scipy integration +- Proper error propagation and uncertainty calculation +- LaTeX output for scientific publication + +## Architecture Weaknesses and Improvements + +### 1. **Metaclass Complexity** +**Issue**: Debugging metaclass interactions can be challenging +**Recommendation**: Add comprehensive logging and better error messages + +### 2. **Abstract Property Deprecation** +**Issue**: `@abstractproperty` deprecated in Python 3.3+ +**Fix Required**: +```python +@property +@abstractmethod +def function(self): + pass +``` + +### 3. **Parameter Bounds Architecture** +**Issue**: Bounds handling is inconsistent across subclasses +**Recommendation**: Implement standardized bounds interface + +### 4. **Error Handling Consistency** +**Issue**: Mixed exception types and error messages +**Recommendation**: Implement custom exception hierarchy + +## Design Pattern Analysis + +### Template Method Pattern Implementation +``` +FitFunction.make_fit(): +ā”œā”€ā”€ sufficient_data check +ā”œā”€ā”€ _run_least_squares() [uses subclass p0, function] +ā”œā”€ā”€ _calc_popt_pcov_psigma_chisq() +ā”œā”€ā”€ build_TeX_info() [uses subclass TeX_function] +└── build_plotter() +``` + +**Assessment**: āœ… **Excellent** - Clear separation of concerns, extensible hooks + +### Factory Pattern Considerations +**Current**: Direct instantiation (`Gaussian(x, y)`) +**Alternative**: Factory method (`FitFunctionFactory.create("gaussian", x, y)`) +**Recommendation**: Keep current - simpler API for scientific users + +### Strategy Pattern for Loss Functions +**Current**: Hardcoded Huber loss in `make_fit()` +**Improvement**: Strategy pattern for different loss functions +**Priority**: Medium - current approach works well + +## Integration with SolarWindPy Architecture + +### 1. **DataFrame Compatibility** +- All fit functions accept numpy arrays +- Compatible with MultiIndex DataFrame `.values` extraction +- No conflicts with M/C/S indexing patterns + +### 2. **Physics Validation** +- Fits integrate with SolarWindPy's unit system +- Proper handling of physical constraints +- Error propagation follows scientific standards + +### 3. **Hook System Integration** +- Pre-commit hooks validate fit function implementations +- Physics validation ensures parameter reasonableness +- Test coverage requirements enforced + +## Recommendations for Phase 3 Implementation + +### Priority 1: Critical Fixes +1. **Fix Abstract Property Deprecation** + - Replace `@abstractproperty` with `@property + @abstractmethod` + - Test all subclass implementations + +2. **Standardize Error Handling** + - Implement `FitFunctionError` exception hierarchy + - Consistent error messages across all classes + +### Priority 2: Architecture Improvements +3. **Implement Parameter Bounds Interface** + - Add `bounds` abstract property + - Standardize bounds format across subclasses + +4. **Enhanced Logging Architecture** + - Structured logging with fit diagnostics + - Debug mode for metaclass operations + +### Priority 3: Documentation Enhancements +5. **Architecture Documentation** + - Developer guide for creating new fit functions + - Metaclass interaction documentation + +6. **Performance Profiling** + - Benchmark fitting performance + - Identify optimization opportunities + +## Testing Architecture + +### Current Coverage +- Unit tests for all 11 fit function classes +- Integration tests with scipy.optimize +- Docstring inheritance validation + +### Recommended Additions +- Metaclass behavior testing +- Error handling edge cases +- Performance regression tests +- Cross-platform compatibility tests + +## Conclusion + +The current fitfunctions architecture successfully implements scientific computing best practices with clean separation of concerns. The metaclass-based docstring inheritance was a bold architectural choice that paid off significantly in terms of code maintainability and documentation consistency. + +The primary focus for Phase 3 should be addressing the technical debt around deprecated APIs and error handling while maintaining the excellent extensibility and consistency achieved in Phases 1-2. + +--- + +**Document Version**: 1.0 +**Last Updated**: 2025-09-08 +**Phase**: 3 - Architecture & Design Pattern Review +**Author**: Claude Code (SolarWindPy Fitfunctions Audit) \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index d593b683..2c78718b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,29 @@ # DO NOT EDIT MANUALLY - regenerate with scripts/freeze_requirements.py # Generated from: requirements-dev.txt +Bottleneck==1.6.0 +PyYAML==6.0.2 +Sphinx==8.2.3 +astropy==7.1.0 +black==25.1.0 doc8==2.0.0 docstring-inheritance==2.2.2 flake8-docstrings==1.7.0 -pytest-cov==6.2.1 +flake8==7.3.0 +gh==0.0.4 +h5py==3.14.0 +matplotlib==3.10.6 +numba==0.61.2 +numexpr==2.11.0 +numpy==2.2.6 +numpydoc==1.9.0 +pandas==2.3.2 +psutil==7.0.0 +pydocstyle==6.3.0 +pytest-cov==6.3.0 +pytest==8.4.2 +scipy==1.16.1 +sphinxcontrib-bibtex==2.6.5 +sphinxcontrib-spelling==8.0.1 +tables==3.10.2 +tabulate==0.9.0 diff --git a/solarwindpy-20250908.yml b/solarwindpy-20250908.yml new file mode 100644 index 00000000..9dca5410 --- /dev/null +++ b/solarwindpy-20250908.yml @@ -0,0 +1,32 @@ +name: solarwindpy-20250908 +channels: +- conda-forge +- defaults +dependencies: +- numpy +- scipy +- pandas +- numexpr +- bottleneck +- h5py +- pyyaml +- matplotlib +- astropy +- numba +- tabulate +- docstring-inheritance>=2.0 +- pytest +- pytest-cov>=4.1.0 +- black +- flake8 +- pytables +- doc8 +- flake8-docstrings +- pydocstyle +- numpydoc +- sphinx +- sphinx_rtd_theme +- sphinxcontrib-spelling +- sphinxcontrib-bibtex +- gh +- psutil>=5.9.0 diff --git a/solarwindpy/fitfunctions/__init__.py b/solarwindpy/fitfunctions/__init__.py index 7dbd30f7..f0e0a5a8 100644 --- a/solarwindpy/fitfunctions/__init__.py +++ b/solarwindpy/fitfunctions/__init__.py @@ -18,3 +18,9 @@ Moyal = moyal.Moyal # Hinge = hinge.Hinge TrendFit = trend_fits.TrendFit + +# Exception classes for better error handling +FitFunctionError = core.FitFunctionError +InsufficientDataError = core.InsufficientDataError +FitFailedError = core.FitFailedError +InvalidParameterError = core.InvalidParameterError diff --git a/solarwindpy/fitfunctions/core.py b/solarwindpy/fitfunctions/core.py index 280232ff..7a249962 100644 --- a/solarwindpy/fitfunctions/core.py +++ b/solarwindpy/fitfunctions/core.py @@ -12,7 +12,7 @@ import warnings import numpy as np -from abc import ABC, abstractproperty +from abc import ABC, abstractmethod from collections import namedtuple from inspect import getfullargspec from docstring_inheritance import NumpyDocstringInheritanceMeta @@ -45,6 +45,30 @@ FitBounds = namedtuple("FitBounds", "lower,upper") +class FitFunctionError(Exception): + """Base exception for fit function errors.""" + + pass + + +class InsufficientDataError(FitFunctionError): + """Raised when there is insufficient data to perform the fit.""" + + pass + + +class FitFailedError(FitFunctionError): + """Raised when the fitting algorithm fails to converge.""" + + pass + + +class InvalidParameterError(FitFunctionError): + """Raised when invalid parameters are provided to fit functions.""" + + pass + + # Combine ABC and docstring inheritance metaclasses class FitFunctionMeta(NumpyDocstringInheritanceMeta, type(ABC)): """Metaclass combining ABC and docstring inheritance.""" @@ -221,7 +245,8 @@ def _init_logger(self): logger = logging.getLogger(f"{__name__}.{self.__class__.__name__}") self._logger = logger - @abstractproperty + @property + @abstractmethod def function(self): r"""Get the function that`curve_fit` fits. @@ -231,12 +256,14 @@ def function(self): """ pass - @abstractproperty + @property + @abstractmethod def p0(self): r"""The initial guess for the FitFunction.""" pass - @abstractproperty + @property + @abstractmethod def TeX_function(self): r"""Function written in LaTeX.""" pass @@ -282,7 +309,6 @@ def fit_result(self): @property def initial_guess_info(self): - # If failed to make an initial guess, then don't build the info. try: p0 = self.p0 @@ -371,7 +397,7 @@ def sufficient_data(self): chk = self.nobs >= len(self.argnames) if not chk: msg = "There is insufficient data to fit the model." - raise ValueError(msg) + raise InsufficientDataError(msg) else: return True @@ -394,17 +420,15 @@ def _clean_raw_obs(self, xobs, yobs, weights): weights = np.asarray(weights) if xobs.shape != yobs.shape: - raise ValueError( - f"""xobs and yobs must have the same shape., -xobs: {xobs.shape}, -yobs: {yobs.shape}""" + raise InvalidParameterError( + f"""xobs and yobs must have the same shape. +xobs: {xobs.shape}, yobs: {yobs.shape}""" ) if weights is not None and weights.shape != xobs.shape: - raise ValueError( - f"""weights and xobs must have the same shape., -weighs: {weights.shape}", -xobs: {xobs.shape}""" + raise InvalidParameterError( + f"""weights and xobs must have the same shape. +weights: {weights.shape}, xobs: {xobs.shape}""" ) return xobs, yobs, weights @@ -474,7 +498,6 @@ def build_plotter(self): return plotter def build_TeX_info(self): - # Allows annotating of TeX_info when fit fails in a manner # that is easily identifiable. try: @@ -660,7 +683,7 @@ def _run_least_squares(self, **kwargs): ) if not res.success: - raise RuntimeError("Optimal parameters not found: " + res.message) + raise FitFailedError("Optimal parameters not found: " + res.message) fit_bounds = np.concatenate([lb, ub]).reshape((2, -1)).T fit_bounds = {k: FitBounds(*b) for k, b in zip(self.argnames, fit_bounds)} @@ -753,11 +776,14 @@ def make_fit(self, return_exception=False, **kwargs): """ try: assert self.sufficient_data # Check we have enough data to fit. - except (AssertionError, ValueError) as e: + except (AssertionError, ValueError, InsufficientDataError) as e: # raise if isinstance(e, AssertionError): - e = ValueError("Insufficient data to fit the model") - return e + e = InsufficientDataError("Insufficient data to fit the model") + if return_exception: + return e + else: + raise absolute_sigma = kwargs.pop("absolute_sigma", False) if absolute_sigma: diff --git a/tests/fitfunctions/test_core.py b/tests/fitfunctions/test_core.py index ff6fc9bb..44877592 100644 --- a/tests/fitfunctions/test_core.py +++ b/tests/fitfunctions/test_core.py @@ -6,6 +6,8 @@ FitFunction, ChisqPerDegreeOfFreedom, InitialGuessInfo, + InvalidParameterError, + InsufficientDataError, ) @@ -29,7 +31,7 @@ def TeX_function(self): def test_clean_raw_obs(): lf = LinearFit([0, 1], [1, 2]) - with pytest.raises(ValueError): + with pytest.raises(InvalidParameterError): lf._clean_raw_obs([0, 1], [1], None) x, y, w = lf._clean_raw_obs([0, 1], [1, 2], [1, 1]) assert np.array_equal(x, np.array([0, 1])) @@ -145,7 +147,7 @@ def test_make_fit_success_failure(monkeypatch, simple_linear_data, small_n): x, y, w = small_n lf_small = LinearFit(x, y, weights=w) err = lf_small.make_fit(return_exception=True) - assert isinstance(err, ValueError) + assert isinstance(err, InsufficientDataError) def fail_run(*_, **__): raise RuntimeError("fail") diff --git a/tests/fitfunctions/test_exponentials.py b/tests/fitfunctions/test_exponentials.py index 5984ff1f..06504398 100644 --- a/tests/fitfunctions/test_exponentials.py +++ b/tests/fitfunctions/test_exponentials.py @@ -9,6 +9,7 @@ ExponentialPlusC, ExponentialCDF, ) +from solarwindpy.fitfunctions.core import InsufficientDataError @pytest.mark.parametrize( @@ -60,7 +61,7 @@ def test_p0_zero_size_input(cls): y = np.array([]) obj = cls(x, y) - with pytest.raises((ValueError, AssertionError)): + with pytest.raises(InsufficientDataError): _ = obj.p0 @@ -171,9 +172,13 @@ def test_make_fit_insufficient_data(): y = np.array([1.0]) obj = cls(x, y) - # By default, make_fit returns exceptions rather than raising them - result = obj.make_fit() - assert isinstance(result, ValueError) + # With insufficient data, make_fit raises InsufficientDataError by default + with pytest.raises(InsufficientDataError): + obj.make_fit() + + # With return_exception=True, make_fit returns the exception + result = obj.make_fit(return_exception=True) + assert isinstance(result, InsufficientDataError) assert "insufficient data" in str(result).lower() # Test ExponentialCDF (needs special setup) @@ -185,10 +190,10 @@ def test_make_fit_insufficient_data(): obj.set_y0(1.0) result = obj.make_fit() - # For ExponentialCDF with 1 point, the fit technically succeeds + # For ExponentialCDF with 1 point, the fit technically succeeds # (1 point for 1 parameter), so it returns None - assert result is None or isinstance(result, ValueError) - if isinstance(result, ValueError): + assert result is None or isinstance(result, InsufficientDataError) + if isinstance(result, InsufficientDataError): assert "insufficient data" in str(result).lower() diff --git a/tests/fitfunctions/test_gaussians.py b/tests/fitfunctions/test_gaussians.py index 58780544..e390bbf1 100644 --- a/tests/fitfunctions/test_gaussians.py +++ b/tests/fitfunctions/test_gaussians.py @@ -7,6 +7,7 @@ GaussianNormalized, GaussianLn, ) +from solarwindpy.fitfunctions.core import InsufficientDataError @pytest.mark.parametrize( @@ -39,7 +40,7 @@ def test_p0_zero_size_input(cls): x = np.array([]) y = np.array([]) obj = cls(x, y) - with pytest.raises(ValueError, match="insufficient data"): + with pytest.raises(InsufficientDataError): _ = obj.p0 diff --git a/tests/fitfunctions/test_lines.py b/tests/fitfunctions/test_lines.py index 65b05c81..b5c76760 100644 --- a/tests/fitfunctions/test_lines.py +++ b/tests/fitfunctions/test_lines.py @@ -8,6 +8,7 @@ Line, LineXintercept, ) +from solarwindpy.fitfunctions.core import InsufficientDataError @pytest.mark.parametrize( @@ -41,7 +42,7 @@ def test_p0_zero_size_input(cls): y = np.array([]) obj = cls(x, y) - with pytest.raises((ValueError, AssertionError)): + with pytest.raises(InsufficientDataError): _ = obj.p0 @@ -120,9 +121,13 @@ def test_make_fit_insufficient_data(cls): y = np.array([1.0]) obj = cls(x, y) - # By default, make_fit returns exceptions rather than raising them - result = obj.make_fit() - assert isinstance(result, ValueError) + # With insufficient data, make_fit raises InsufficientDataError by default + with pytest.raises(InsufficientDataError): + obj.make_fit() + + # With return_exception=True, make_fit returns the exception + result = obj.make_fit(return_exception=True) + assert isinstance(result, InsufficientDataError) assert "insufficient data" in str(result).lower() diff --git a/tests/fitfunctions/test_moyal.py b/tests/fitfunctions/test_moyal.py index f430aff6..872ab844 100644 --- a/tests/fitfunctions/test_moyal.py +++ b/tests/fitfunctions/test_moyal.py @@ -5,6 +5,7 @@ import pytest from solarwindpy.fitfunctions.moyal import Moyal +from solarwindpy.fitfunctions.core import InsufficientDataError @pytest.mark.parametrize( @@ -64,8 +65,8 @@ def test_p0_zero_size_input(cls): y = np.array([]) obj = cls(x, y) # xobs, yobs - # Should raise ValueError due to insufficient data (from sufficient_data property) - with pytest.raises(ValueError, match="insufficient data"): + # Should raise InsufficientDataError due to insufficient data (from sufficient_data property) + with pytest.raises(InsufficientDataError): _ = obj.p0 @@ -89,7 +90,10 @@ def test_moyal_p0_estimation(moyal_data): @pytest.mark.parametrize( "cls, expected_tex", [ - (Moyal, r"f(x) = A \cdot \exp\left[\frac{1}{2}\left(\left(\frac{x-\mu}{\sigma}\right)^2 - \exp\left(\left(\frac{x-\mu}{\sigma}\right)^2\right)\right)\right]"), # Fixed LaTeX formula + ( + Moyal, + r"f(x) = A \cdot \exp\left[\frac{1}{2}\left(\left(\frac{x-\mu}{\sigma}\right)^2 - \exp\left(\left(\frac{x-\mu}{\sigma}\right)^2\right)\right)\right]", + ), # Fixed LaTeX formula ], ) def test_TeX_function_strings(cls, expected_tex): @@ -128,8 +132,8 @@ def test_make_fit_insufficient_data(): obj = Moyal(x, y) # xobs, yobs - # Should raise ValueError when accessing sufficient_data - with pytest.raises(ValueError, match="insufficient data"): + # Should raise InsufficientDataError when accessing sufficient_data + with pytest.raises(InsufficientDataError): _ = obj.sufficient_data @@ -227,7 +231,7 @@ def test_moyal_constructor_issues(): # Test that the sigma parameter is not actually used properly # (the implementation has commented out the sigma usage) try: - sigma_prop = obj.sigma + _ = obj.sigma # Don't store unused variable # This will likely fail since _sigma is not set except AttributeError: # Expected due to broken implementation diff --git a/tests/fitfunctions/test_power_laws.py b/tests/fitfunctions/test_power_laws.py index a98f0bec..e41b9b43 100644 --- a/tests/fitfunctions/test_power_laws.py +++ b/tests/fitfunctions/test_power_laws.py @@ -9,6 +9,7 @@ PowerLawPlusC, PowerLawOffCenter, ) +from solarwindpy.fitfunctions.core import InsufficientDataError @pytest.mark.parametrize( @@ -56,7 +57,7 @@ def test_p0_zero_size_input(cls): y = np.array([]) obj = cls(x, y) - with pytest.raises((ValueError, AssertionError)): + with pytest.raises(InsufficientDataError): _ = obj.p0 @@ -138,9 +139,13 @@ def test_make_fit_insufficient_data(cls): y = np.array([1.0]) obj = cls(x, y) - # By default, make_fit returns exceptions rather than raising them - result = obj.make_fit() - assert isinstance(result, ValueError) + # With insufficient data, make_fit raises InsufficientDataError by default + with pytest.raises(InsufficientDataError): + obj.make_fit() + + # With return_exception=True, make_fit returns the exception + result = obj.make_fit(return_exception=True) + assert isinstance(result, InsufficientDataError) assert "insufficient data" in str(result).lower() diff --git a/tests/fitfunctions/test_trend_fits.py b/tests/fitfunctions/test_trend_fits.py index 63d904e3..6a11ebb6 100644 --- a/tests/fitfunctions/test_trend_fits.py +++ b/tests/fitfunctions/test_trend_fits.py @@ -6,7 +6,7 @@ import pytest from scipy.optimize import OptimizeWarning -from solarwindpy.fitfunctions import core, gaussians, trend_fits, lines +from solarwindpy.fitfunctions import gaussians, trend_fits, lines from solarwindpy.fitfunctions.plots import AxesLabels @@ -134,10 +134,10 @@ def errorbar(self, *_, **__): def legend(self, *_, **__): pass - + def plot(self, *args, **kwargs): return None - + def fill_between(self, *args, **kwargs): return None @@ -157,9 +157,7 @@ def fill_between(self, *args, **kwargs): monkeypatch.setattr( trend_fit.trend_func.plotter, "plot_raw_used_fit", lambda *_, **__: None ) - monkeypatch.setattr( - trend_fits, "subplots", lambda *_, **__: (None, DummyAx()) - ) + monkeypatch.setattr(trend_fits, "subplots", lambda *_, **__: (None, DummyAx())) ax = trend_fit.plot_1d_popt_and_trend() assert isinstance(ax, DummyAx) @@ -177,11 +175,11 @@ def errorbar(self, *args, **kwargs): def set_xscale(self, *args, **kwargs): # pragma: no cover - not used here self.calls["set_xscale"] = {"args": args, "kwargs": kwargs} - + def plot(self, *args, **kwargs): self.calls["plot"] = {"args": args, "kwargs": kwargs} return None - + def fill_between(self, *args, **kwargs): self.calls["fill_between"] = {"args": args, "kwargs": kwargs} return None @@ -192,7 +190,9 @@ def fill_between(self, *args, **kwargs): tf.make_trend_func() ax = StubAx() - pl, cl, bl = tf.plot_all_popt_1d(ax, color="magenta", label="1D Fits", plot_window=False) + pl, cl, bl = tf.plot_all_popt_1d( + ax, color="magenta", label="1D Fits", plot_window=False + ) assert (pl, cl, bl) == ax.ret @@ -241,4 +241,6 @@ def test_labels_instance_and_update(trend_fit): # Labels are stored in the trend_func's plotter, not in TrendFit itself assert isinstance(trend_fit.trend_func.plotter.labels, AxesLabels) trend_fit.set_shared_labels(x="time", y="density", z="counts") - assert trend_fit.trend_func.plotter.labels == AxesLabels("time", "density", "counts") + assert trend_fit.trend_func.plotter.labels == AxesLabels( + "time", "density", "counts" + )