-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(shared-data): Fix migrating mutable pipette configs #16962
fix(shared-data): Fix migrating mutable pipette configs #16962
Conversation
elif thiskey == "##EACHTIP##": | ||
for key in existing.keys(): | ||
_do_edit_non_quirk(new_value, existing[key], restkeys) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this other function need the same change?
opentrons/shared-data/python/opentrons_shared_data/pipette/load_data.py
Lines 169 to 200 in efb4f42
def _edit_non_quirk_with_lc_override( | |
mutable_config_key: str, | |
new_mutable_value: Any, | |
base_dict: Dict[str, Any], | |
liquid_class: Optional[LiquidClasses], | |
) -> None: | |
def _do_edit_non_quirk( | |
new_value: Any, existing: Dict[Any, Any], keypath: List[Any] | |
) -> None: | |
thiskey: Any = keypath[0] | |
if thiskey in [lc.name for lc in LiquidClasses]: | |
if liquid_class: | |
thiskey = liquid_class | |
else: | |
thiskey = LiquidClasses[thiskey] | |
if len(keypath) > 1: | |
restkeys = keypath[1:] | |
if thiskey == "##EACHTIP##": | |
for key in existing.keys(): | |
_do_edit_non_quirk(new_value, existing[key], restkeys) | |
else: | |
_do_edit_non_quirk(new_value, existing[thiskey], restkeys) | |
else: | |
# This was the last key | |
if thiskey == "##EACHTIP##": | |
for key in existing.keys(): | |
existing[key] = new_value | |
else: | |
existing[thiskey] = new_value | |
new_names = _MAP_KEY_TO_V2[mutable_config_key] | |
_do_edit_non_quirk(new_mutable_value, base_dict, new_names) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me. very confused though
Overview
Closes RQA-3676.
Test Plan and Hands on Testing
Changelog
Review requests
Is there a more useful test to check that could check the correctness of the result, not merely that no exceptions are logged?
Risk assessment
I (Max) think this is high-risk because I understand maybe 40% of it. Those more familiar with the code might find it less worrying.