Conversation
rozyczko
added
[scope] maintenance
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## easylist-on-multifitter #306 +/- ##
===========================================================
+ Coverage 83.66% 83.81% +0.14%
===========================================================
Files 68 68
Lines 5271 5332 +61
===========================================================
+ Hits 4410 4469 +59
- Misses 861 863 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
rozyczko
changed the title
damskii9992
left a comment
damskii9992
left a comment
There was a problem hiding this comment.
More work to do. You forgot the biggest part of the change, the removal of the name attribute.
|
|
||
| def __init__( | ||
| self, | ||
| name: str, |
There was a problem hiding this comment.
We want to get rid of the name attribute. This was part of the reason to move to NewBase.
| # Only ModelBase members are accepted: EasyList harvests parameters | ||
| # from ModelBase items alone, so any other NewBase (a bare Parameter, | ||
| # say) would be accepted and then silently sit out the fit. | ||
| self._fit_objects = EasyList(*fit_objects, protected_types=ModelBase) |
There was a problem hiding this comment.
Hmm, I guess we would need to edit the EasyList to also return bare Parameters, otherwise we can't get rid of ObjBase to use the Fitter in standalone fitting.
There was a problem hiding this comment.
or maybe have Fitter.__init__ accept a plain list (or tuple) of parameters and wrap it in an EasyList itself? so users don't even need to know the container exists.
There was a problem hiding this comment.
Even better. But we will still need to edit the EasyList I think, otherwise the fitter won't be able to get the parameters, right?
There was a problem hiding this comment.
Moved out to separate issue
I prefer to do it in a separate PR. The changes required are quite substantial. |
damskii9992
left a comment
damskii9992
left a comment
There was a problem hiding this comment.
I am happy with all the changes regarding the DescriptorBase now, I still think we need to add the small EasyList change here, or in the other PR, so that the get_all_parameters method also return bare Parameters if they exist in the list, so that EasyList truly can replace ObjBase.
|
|
2 participants
DescriptorBasenow inherits fromNewBaseinstead ofSerializerComponent, and usesSerializerBasefor serialization. Theto_dictmethod is introduced as the standard serialization method, withas_dictprovided as an alias for backward compatibility.The serialization interface for descriptor and parameter classes (
DescriptorNumber,DescriptorArray,DescriptorAnyType,Parameter) is unified to useto_dict, replacing previousas_dictimplementations.EasyListused inMultiFitternow only acceptsModelBasemembers, preventing bare parameters or foreign objects from being silently ignored during fitting.