Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(shared-data): Fix migrating mutable pipette configs #16962

Merged
merged 7 commits into from
Nov 22, 2024
Merged

fix(shared-data): Fix migrating mutable pipette configs #16962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
75d9112
Fix file leak in test.
SyntaxColoring Nov 22, 2024
f6250a7
Add quick and dirty tests.
SyntaxColoring Nov 22, 2024
98358bb
Comments.
SyntaxColoring Nov 22, 2024
efb4f42
Seth's fix.
SyntaxColoring Nov 22, 2024
193f113
i think this is it
sfoster1 Nov 22, 2024
721be16
noqa
SyntaxColoring Nov 22, 2024
3ab5695
Okay I guess the previous commit means we're reverting the original fix?
SyntaxColoring Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion shared-data/python/opentrons_shared_data/pipette/mutable_configurations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
LIQUID_CLASS = LiquidClasses.default


def _edit_non_quirk(
def _edit_non_quirk( # noqa: C901
mutable_config_key: str, new_mutable_value: MutableConfig, base_dict: Dict[str, Any]
) -> None:
def _do_edit_non_quirk(
Expand All @@ -58,6 +58,9 @@ def _do_edit_non_quirk(
elif thiskey == "##EACHTIPTYPE##":
for key in existing.keys():
_do_edit_non_quirk(new_value, existing[key], restkeys)
elif thiskey == "##EACHTIP##":
for key in existing.keys():
_do_edit_non_quirk(new_value, existing[key], restkeys)
Comment on lines +61 to +63
Copy link
Contributor Author

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)

else:
_do_edit_non_quirk(new_value, existing[thiskey], restkeys)
else:
Expand Down
162 changes: 159 additions & 3 deletions shared-data/python/tests/pipette/test_mutable_configurations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import logging
import os
from pathlib import Path
from typing import Dict, Any, cast, Union, Generator
Expand Down Expand Up @@ -68,9 +69,8 @@ def test_load_old_overrides_regression(
"type": "float",
"default": 0.1,
}
json.dump(
TMPFILE_DATA, open(override_configuration_path / "P20SV222021040709.json", "w")
)
with open(override_configuration_path / "P20SV222021040709.json", "w") as f:
json.dump(TMPFILE_DATA, f)
configs = mutable_configurations.load_with_mutable_configurations(
pipette_definition.PipetteModelVersionType(
pipette_type=types.PipetteModelType.p20,
Expand Down Expand Up @@ -305,3 +305,159 @@ def test_build_mutable_config_using_old_units() -> None:
assert (
types.MutableConfig.build(**old_units_config, name="dropTipSpeed") is not None # type: ignore
)


@pytest.mark.parametrize(
("filename", "type", "channels", "version", "file_contents"),
# From https://opentrons.atlassian.net/browse/RQA-3676.
# These could probably be pared down.
[
(
"P20MV202020121412.json",
types.PipetteModelType.p20,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p20_multi_v2.0"}',
),
(
"P3HSV1318071638.json",
types.PipetteModelType.p300,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(1, 3),
'{"dropTipShake": true, "model": "p300_single_v1.3", "quirks": {"dropTipShake": true}, "top": {"value": 30.0, "default": 19.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "pickUpPresses": {"value": 3.0, "default": 3, "units": "presses", "type": "int", "min": 0, "max": 15}}',
),
(
"P3HMV212021040004.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 1),
'{"needsUnstick": true, "model": "p300_multi_v2.1"}',
),
(
"P20SV202020032604.json",
types.PipetteModelType.p20,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p20_single_v2.0"}',
),
(
"P1KSV202019072441.json",
types.PipetteModelType.p1000,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"pickupTipShake": true, "model": "p1000_single_v2.0", "quirks": {"pickupTipShake": true}}',
),
(
"P3HMV202021011105.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 0),
'{"needsUnstick": true, "model": "p300_multi_v2.0"}',
),
(
"P20SV202019072527.json",
types.PipetteModelType.p20,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p20_single_v2.0"}',
),
(
"P3HSV202021042602.json",
types.PipetteModelType.p300,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p300_single_v2.0"}',
),
(
"P20SV222021030914.json",
types.PipetteModelType.p20,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 2),
'{"model": "p20_single_v2.2", "quirks": {}, "pickUpPresses": {"value": 3.0, "default": 1, "units": "presses", "type": "int", "min": 0, "max": 15}}',
),
(
"P3HMV202020040801.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 0),
'{"needsUnstick": true, "model": "p300_multi_v2.0", "quirks": {"needsUnstick": true}, "top": {"value": 20.0, "default": 19.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "bottom": {"value": -14.0, "default": -14.5, "units": "mm", "type": "float", "min": -20, "max": 30}, "blowout": {"value": -18.5, "default": -19.0, "units": "mm", "type": "float", "min": -20, "max": 30}, "dropTip": {"value": -20.0, "default": -33.4, "units": "mm", "type": "float", "min": -20, "max": 30}, "pickUpCurrent": {"value": 0.9, "default": 0.8, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "pickUpDistance": {"value": 10.0, "default": 11.0, "units": "mm", "type": "float", "min": 0, "max": 10}, "pickUpIncrement": {"value": 0.1, "default": 0.0, "units": "mm", "type": "int", "min": 0, "max": 10}, "pickUpPresses": {"value": 4.0, "default": 1, "units": "presses", "type": "int", "min": 0, "max": 15}, "pickUpSpeed": {"value": 11.0, "default": 10.0, "units": "mm/s", "type": "float", "min": 0.01, "max": 30}, "plungerCurrent": {"value": 1.1, "default": 1.0, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "dropTipCurrent": {"value": 1.22, "default": 1.25, "units": "amps", "type": "float", "min": 0.1, "max": 2.0}, "dropTipSpeed": {"value": 8.0, "default": 7.5, "units": "mm/s", "type": "float", "min": 0.01, "max": 30}, "tipLength": {"value": 52.0, "default": 51.0, "units": "mm", "type": "float", "min": 0, "max": 100}}',
),
(
"P3HMV212021040002.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 1),
'{"needsUnstick": true, "model": "p300_multi_v2.1"}',
),
(
"P3HMV1318072625.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(1, 3),
'{"dropTipShake": true, "model": "p300_multi_v1.3", "quirks": {"dropTipShake": true}, "pickUpPresses": {"value": 4.0, "default": 3, "units": "presses", "type": "int", "min": 0, "max": 15}}',
),
(
"P50MV1519091757.json",
types.PipetteModelType.p50,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(1, 5),
'{"dropTipShake": true, "doubleDropTip": true, "model": "p50_multi_v1.5", "quirks": {"doubleDropTip": true, "dropTipShake": true}}',
),
(
"P3HSV202019072224.json",
types.PipetteModelType.p300,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p300_single_v2.0", "quirks": {}}',
),
(
"P20MV202019112708.json",
types.PipetteModelType.p20,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 0),
'{"model": "p20_multi_v2.0", "quirks": {}}',
),
(
"P3HSV202021031503.json",
types.PipetteModelType.p300,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 1),
'{"model": "p300_single_v2.1"}',
),
(
"P1KSV202020060206.json",
types.PipetteModelType.p1000,
types.PipetteChannelType.SINGLE_CHANNEL,
types.PipetteVersionType(2, 0),
'{"pickupTipShake": true, "model": "p1000_single_v2.0"}',
),
(
"P3HMV202021010906.json",
types.PipetteModelType.p300,
types.PipetteChannelType.EIGHT_CHANNEL,
types.PipetteVersionType(2, 0),
'{"needsUnstick": true, "model": "p300_multi_v2.0"}',
),
],
)
def test_loading_does_not_log_warnings(
filename: str,
type: types.PipetteModelType,
channels: types.PipetteChannelType,
version: types.PipetteVersionType,
file_contents: str,
caplog: pytest.LogCaptureFixture,
override_configuration_path: Path,
) -> None:
"""Make sure load_with_mutable_configurations() doesn't log any exceptions.

load_with_mutable_configurations() suppresses and logs internal exceptions to
protect its caller, but those are still bugs, and we still want tests to catch them.
"""
model = pipette_definition.PipetteModelVersionType(type, channels, version)
(override_configuration_path / filename).write_text(file_contents)
with caplog.at_level(logging.WARNING):
mutable_configurations.load_with_mutable_configurations(
model, override_configuration_path, Path(filename).stem
)
assert len(caplog.records) == 0
Loading