Add ParameterAnalysis#167
Conversation
henrikjacobsenfys
added
[scope] enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #167 +/- ##
===========================================
+ Coverage 98.12% 98.18% +0.06%
===========================================
Files 45 47 +2
Lines 2874 3137 +263
Branches 511 568 +57
===========================================
+ Hits 2820 3080 +260
- Misses 34 35 +1
- Partials 20 22 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
rozyczko
left a comment
rozyczko
left a comment
There was a problem hiding this comment.
Well written functionality. Just a few minor comments
| if not _in_notebook(): | ||
| raise RuntimeError('plot() can only be used in a Jupyter notebook environment.') |
There was a problem hiding this comment.
This makes testing the plot function very awkward (tests must mock _in_notebook). Consider whether this restriction is necessary. Doesn't plopp support non-notebook figures? Maybe it can be used without the notebook.
There was a problem hiding this comment.
The plopp slicer is only supported in notebooks. I agree it's very awkward, but the alternative is not having the slicer and then the logic gets complicated since I need to figure out how to plot the data then.
| def bindings(self) -> list[FitBinding] | None: | ||
| """ |
There was a problem hiding this comment.
self._binding is always a list
def _verify_bindings(self, bindings: FitBinding | list[FitBinding] | None) -> list[FitBinding]:so | None is wrong here.
| FIT_FUNCTION_TYPE = ModelComponent | ComponentCollection | DiffusionModelBase | ||
|
|
There was a problem hiding this comment.
This seems to be unused. Consider removing
| Can be used to fit paramters to ModelComponents, ComponentCollections, or DiffusionModelBase | ||
| objects, and to plot the parameters and fit results. The parameters to be analyzed can be |
|
@AndrewSazonov Do you know why these tests fail? All I did was merge develop (tests failed), pixi update and pixi run fix... The tests worked on the branch that was merged into develop, so I have no idea what went wrong.. |
2 participants
To fit the fit result from
Analysisto either aModelComponent,ComponentCollectionorDiffusionModel.