fix(dap): lmfit1d can pass any parameters with kwargs#707
fix(dap): lmfit1d can pass any parameters with kwargs#707wyzula-jan wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the lmfit1d service to accept generic fitting parameters through a flexible kwargs-based API. The main improvement allows users to pass any valid model parameters via a parameters dictionary argument, with automatic validation and filtering of invalid parameter names.
Key Changes:
- Added a new
parameterskwarg to theconfiguremethod that accepts either dict or lmfit.Parameters objects - Implemented parameter validation logic that filters out invalid parameters for the selected model
- Enhanced error handling and logging in the fit process with detailed parameter tracking
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| bec_server/bec_server/data_processing/lmfit1d_service.py | Core implementation of flexible parameter handling, validation, and filtering logic; added error handling with logging for fit execution |
| bec_lib/bec_lib/lmfit_serializer.py | Updated deserialize_param_object to handle both dict and Parameter object inputs; changed return type annotation from Parameter to Parameters |
| bec_server/tests/tests_data_processing/test_lmfit1d_service.py | Added tests for generic parameter passing and validation filtering across different model types (GaussianModel and SineModel) |
| bec_lib/tests/test_lmfit_serializer.py | Added test case to verify deserialization works with optional "name" field in parameter dictionaries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
809f72b to
671fb2f
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
671fb2f to
f4241c5
Compare
3398749 to
1215b97
Compare
1215b97 to
238d5fc
Compare
fb0513b to
c595215
Compare
wakonig
left a comment
There was a problem hiding this comment.
In general, it looks good to me. I would suggest to refactor the long methods a bit to improve the readability and avoid the deep nesting.
c595215 to
39cffe9
Compare
39cffe9 to
1e9767b
Compare
1e9767b to
bfd10be
Compare
Adaptation that LMFIT parameters kwargs can be passed for lmfit1d_service, required for bec-project/bec_widgets#987 ; requested by cSAXS
One can now pass multiple models for more complex fitting such as multiple Gaussians, fit background etc. Check bec-project/bec_widgets#987 where are examples and showcase of high level API in
waveform.py