Curve fit base - #54
Conversation
Not up to standards ⛔
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #54 +/- ##
===========================================
- Coverage 98.69% 84.15% -14.54%
===========================================
Files 9 10 +1
Lines 229 309 +80
Branches 26 33 +7
===========================================
+ Hits 226 260 +34
- Misses 2 48 +46
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| @@ -1,2 +1,3 @@ | |||
| numpy==2.2.6 | |||
| matplotlib==3.10.3 | |||
| scipy>=1.7.3 | |||
There was a problem hiding this comment.
I'm not sure about this because scipy==1.7.3 is the latest version that supports Python 3.7, but it only supports Python versions up to 3.10. What should we do in this case?
There was a problem hiding this comment.
There is no need to be concerned. As you may know, we do not directly install this fixed version, and this file is intended solely for triggering Dependabot. PyPI automatically installs the latest available version of each dependency across different Python versions.
|
|
||
| ## [Unreleased] | ||
| ### Added | ||
| - `CurveFit` object |
| ) | ||
|
|
||
| # Single point of registration mapping a model name to its class. | ||
| MODEL_CLASSES: Dict[str, Type[DrugReleaseModel]] = { |
There was a problem hiding this comment.
Don't use typing for internal variables.
| if self._fit_result is None: | ||
| return f"drux.CurveFit({self._model_name}, not fitted)" | ||
| params = ", ".join(f"{k}={v:.4f}" for k, v in self._fit_result.parameters.items()) | ||
| return f"drux.CurveFit({self._model_name}: {params}, R²={self._fit_result.r_squared:.4f})" |
There was a problem hiding this comment.
Don't use Unicode superscript ² (R²) in this function.
| for name in self._parameter_names | ||
| } | ||
|
|
||
| def _equation(self, t: np.ndarray, *free_values: float) -> np.ndarray: |
There was a problem hiding this comment.
Add docstring for inputs.
:param t: ...
:param free_values: ...
|
|
||
| self._fit_result: Optional[FitResult] = None | ||
|
|
||
| def _merge_parameters(self, free_values: Sequence[float]) -> Dict[str, float]: |
There was a problem hiding this comment.
Add docstring for inputs.
| } | ||
|
|
||
|
|
||
| def _model_parameter_names(model_class: Type[DrugReleaseModel]) -> List[str]: |
There was a problem hiding this comment.
Add docstring for inputs.
| ERROR_INVALID_GEOMETRY_FACTOR = "Geometry factor (n) must be 1 (slab), 2 (cylinder), or 3 (sphere)." | ||
|
|
||
| # Error messages for curve fitting | ||
| ERROR_UNKNOWN_MODEL = "Unknown model '{}'. Available models: {}." |
There was a problem hiding this comment.
I strongly suggest using names in formatting.
For example:
ERROR_UNKNOWN_MODEL = "Unknown model '{model_name}'. Available models: {available_models}."And then:
ERROR_UNKNOWN_MODEL.format(model_name=model_name, available_models=available_models)| ## [Unreleased] | ||
| ### Added | ||
| - `CurveFit` class | ||
| - `FitResult` object |
| for name in self._parameter_names | ||
| } | ||
|
|
||
| def _equation(self, t: np.ndarray, *free_values: float) -> np.ndarray: |
There was a problem hiding this comment.
Are you sure about the type of *free_values?
What does this implement?
CurveFitclass is added