Closed apchytr closed 3 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 89.10%. Comparing base (
877d0ea
) to head (77e2690
). Report is 1 commits behind head on develop.
I like it! I don't wanna drag this out much so I'll give my approval, but I do have an alternate proposal. Perhaps instead of moving
_getitem_builtin
up, we should just add the functionality toParameterSet
- something liketodict()
? then themodes
quirk can have its own handling in one place, and the parameter kwargs generation can be in another place. thoughts? if you do choose to make the change, I promise to come back with another quick approval 😇
@timmysilv pushed a to_dict
method on ParameterSet
. It's mostly just taking the kwargs logic from the previous _getitem_builtin
👍 feel free to take a look!
Context: Atm calling e.g
DGate.to_fock
returns aUnitary
instead of aDGate
due to_from_attributes
. Here is a proposed alternative where we attempt to initialize the original class using theParameterSet
and default to_from_attributes
if that fails.Description of the Change:
to_fock
will attempt to initializeself.__class__
using_getitem_builtin
(moved fromState
) and if it fails with aTypeError
will default to_from_attributes
.Number
andSauron
now populate their parameter sets.ParameterSet
now has ato_dict
method. Lots of code cleanup/ appeasing codefactor/code coverage.Benefits:
to_fock
returns more user facing classes. This helps us avoid the need to overwrite_from_attributes
in some cases.