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.

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

feat(shared-data, api): remove 'default' from all '..ByVolume' properties #16878

Merged
24 changes: 10 additions & 14 deletions api/src/opentrons/protocol_api/_liquid_properties.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from dataclasses import dataclass
from numpy import interp
from typing import Optional, Dict, Sequence, Union, Tuple
from typing import Optional, Dict, Sequence, Tuple

from opentrons_shared_data.liquid_classes.liquid_class_definition import (
AspirateProperties as SharedDataAspirateProperties,
Expand All @@ -23,31 +23,27 @@


class LiquidHandlingPropertyByVolume:
def __init__(self, properties_by_volume: Dict[str, float]) -> None:
self._default = properties_by_volume["default"]
def __init__(self, by_volume_property: Sequence[Tuple[float, float]]) -> None:
self._properties_by_volume: Dict[float, float] = {
float(volume): value
for volume, value in properties_by_volume.items()
if volume != "default"
float(volume): value for volume, value in by_volume_property
}
# Volumes need to be sorted for proper interpolation of non-defined volumes, and the
# corresponding values need to be in the same order for them to be interpolated correctly
self._sorted_volumes: Tuple[float, ...] = ()
self._sorted_values: Tuple[float, ...] = ()
self._sort_volume_and_values()

@property
def default(self) -> float:
"""Get the default value not associated with any volume for this property."""
return self._default

def as_dict(self) -> Dict[Union[float, str], float]:
def as_dict(self) -> Dict[float, float]:
"""Get a dictionary representation of all set volumes and values along with the default."""
return self._properties_by_volume | {"default": self._default}
return self._properties_by_volume

def get_for_volume(self, volume: float) -> float:
"""Get a value by volume for this property. Volumes not defined will be interpolated between set volumes."""
validated_volume = validation.ensure_positive_float(volume)
if len(self._properties_by_volume) == 0:
raise ValueError(
"No properties found for any volumes. Cannot interpolate for the given volume."
)
try:
return self._properties_by_volume[validated_volume]
except KeyError:
Expand All @@ -66,9 +62,9 @@ def delete_for_volume(self, volume: float) -> None:
"""Remove an existing volume and value from the property."""
try:
del self._properties_by_volume[volume]
self._sort_volume_and_values()
except KeyError:
raise KeyError(f"No value set for volume {volume} uL")
self._sort_volume_and_values()

def _sort_volume_and_values(self) -> None:
"""Sort volume in increasing order along with corresponding values in matching order."""
Expand Down
14 changes: 7 additions & 7 deletions api/tests/opentrons/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,10 +804,10 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
namespace="test-fixture-2",
byPipette=[
ByPipetteSetting(
pipetteModel="p20_single_gen2",
pipetteModel="flex_1channel_50",
byTipType=[
ByTipTypeSetting(
tiprack="opentrons_96_tiprack_20ul",
tiprack="opentrons_flex_96_tiprack_50ul",
aspirate=AspirateProperties(
submerge=Submerge(
positionReference=PositionReference.LIQUID_MENISCUS,
Expand All @@ -821,13 +821,13 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
positionReference=PositionReference.WELL_TOP,
offset=Coordinate(x=0, y=0, z=5),
speed=100,
airGapByVolume={"default": 2, "5": 3, "10": 4},
airGapByVolume=[(5.0, 3.0), (10.0, 4.0)],
touchTip=TouchTipProperties(enable=False),
delay=DelayProperties(enable=False),
),
positionReference=PositionReference.WELL_BOTTOM,
offset=Coordinate(x=0, y=0, z=-5),
flowRateByVolume={"default": 50, "10": 40, "20": 30},
flowRateByVolume=[(10.0, 40.0), (20.0, 30.0)],
preWet=True,
mix=MixProperties(enable=False),
delay=DelayProperties(
Expand All @@ -845,16 +845,16 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
positionReference=PositionReference.WELL_TOP,
offset=Coordinate(x=0, y=0, z=5),
speed=100,
airGapByVolume={"default": 2, "5": 3, "10": 4},
airGapByVolume=[(5.0, 3.0), (10.0, 4.0)],
blowout=BlowoutProperties(enable=False),
touchTip=TouchTipProperties(enable=False),
delay=DelayProperties(enable=False),
),
positionReference=PositionReference.WELL_BOTTOM,
offset=Coordinate(x=0, y=0, z=-5),
flowRateByVolume={"default": 50, "10": 40, "20": 30},
flowRateByVolume=[(10.0, 40.0), (20.0, 30.0)],
mix=MixProperties(enable=False),
pushOutByVolume={"default": 5, "10": 7, "20": 10},
pushOutByVolume=[(10.0, 7.0), (20.0, 10.0)],
delay=DelayProperties(enable=False),
),
multiDispense=None,
Expand Down
7 changes: 3 additions & 4 deletions api/tests/opentrons/protocol_api/test_liquid_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ def test_get_for_pipette_and_tip(
) -> None:
"""It should get the properties for the specified pipette and tip."""
liq_class = LiquidClass.create(minimal_liquid_class_def2)
result = liq_class.get_for("p20_single_gen2", "opentrons_96_tiprack_20ul")
result = liq_class.get_for("flex_1channel_50", "opentrons_flex_96_tiprack_50ul")
assert result.aspirate.flow_rate_by_volume.as_dict() == {
"default": 50.0,
10.0: 40.0,
20.0: 30.0,
}
Expand All @@ -36,7 +35,7 @@ def test_get_for_raises_for_incorrect_pipette_or_tip(
liq_class = LiquidClass.create(minimal_liquid_class_def2)

with pytest.raises(ValueError):
liq_class.get_for("p20_single_gen2", "no_such_tiprack")
liq_class.get_for("flex_1channel_50", "no_such_tiprack")

with pytest.raises(ValueError):
liq_class.get_for("p300_single", "opentrons_96_tiprack_20ul")
liq_class.get_for("no_such_pipette", "opentrons_flex_96_tiprack_50ul")
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

def test_build_aspirate_settings() -> None:
"""It should convert the shared data aspirate settings to the PAPI type."""
fixture_data = load_shared_data("liquid-class/fixtures/fixture_glycerol50.json")
fixture_data = load_shared_data("liquid-class/fixtures/1/fixture_glycerol50.json")
liquid_class_model = LiquidClassSchemaV1.parse_raw(fixture_data)
aspirate_data = liquid_class_model.byPipette[0].byTipType[0].aspirate

Expand All @@ -32,7 +32,6 @@ def test_build_aspirate_settings() -> None:
assert aspirate_properties.retract.offset == Coordinate(x=0, y=0, z=5)
assert aspirate_properties.retract.speed == 100
assert aspirate_properties.retract.air_gap_by_volume.as_dict() == {
"default": 2.0,
5.0: 3.0,
10.0: 4.0,
}
Expand All @@ -45,7 +44,7 @@ def test_build_aspirate_settings() -> None:

assert aspirate_properties.position_reference.value == "well-bottom"
assert aspirate_properties.offset == Coordinate(x=0, y=0, z=-5)
assert aspirate_properties.flow_rate_by_volume.as_dict() == {"default": 50.0}
assert aspirate_properties.flow_rate_by_volume.as_dict() == {10: 50.0}
assert aspirate_properties.pre_wet is True
assert aspirate_properties.mix.enabled is True
assert aspirate_properties.mix.repetitions == 3
Expand All @@ -56,7 +55,7 @@ def test_build_aspirate_settings() -> None:

def test_build_single_dispense_settings() -> None:
"""It should convert the shared data single dispense settings to the PAPI type."""
fixture_data = load_shared_data("liquid-class/fixtures/fixture_glycerol50.json")
fixture_data = load_shared_data("liquid-class/fixtures/1/fixture_glycerol50.json")
liquid_class_model = LiquidClassSchemaV1.parse_raw(fixture_data)
single_dispense_data = liquid_class_model.byPipette[0].byTipType[0].singleDispense

Expand All @@ -75,7 +74,6 @@ def test_build_single_dispense_settings() -> None:
assert single_dispense_properties.retract.offset == Coordinate(x=0, y=0, z=5)
assert single_dispense_properties.retract.speed == 100
assert single_dispense_properties.retract.air_gap_by_volume.as_dict() == {
"default": 2.0,
5.0: 3.0,
10.0: 4.0,
}
Expand All @@ -93,15 +91,13 @@ def test_build_single_dispense_settings() -> None:
assert single_dispense_properties.position_reference.value == "well-bottom"
assert single_dispense_properties.offset == Coordinate(x=0, y=0, z=-5)
assert single_dispense_properties.flow_rate_by_volume.as_dict() == {
"default": 50.0,
10.0: 40.0,
20.0: 30.0,
}
assert single_dispense_properties.mix.enabled is True
assert single_dispense_properties.mix.repetitions == 3
assert single_dispense_properties.mix.volume == 15
assert single_dispense_properties.push_out_by_volume.as_dict() == {
"default": 5.0,
10.0: 7.0,
20.0: 10.0,
}
Expand All @@ -111,7 +107,7 @@ def test_build_single_dispense_settings() -> None:

def test_build_multi_dispense_settings() -> None:
"""It should convert the shared data multi dispense settings to the PAPI type."""
fixture_data = load_shared_data("liquid-class/fixtures/fixture_glycerol50.json")
fixture_data = load_shared_data("liquid-class/fixtures/1/fixture_glycerol50.json")
liquid_class_model = LiquidClassSchemaV1.parse_raw(fixture_data)
multi_dispense_data = liquid_class_model.byPipette[0].byTipType[0].multiDispense

Expand All @@ -131,7 +127,6 @@ def test_build_multi_dispense_settings() -> None:
assert multi_dispense_properties.retract.offset == Coordinate(x=0, y=0, z=5)
assert multi_dispense_properties.retract.speed == 100
assert multi_dispense_properties.retract.air_gap_by_volume.as_dict() == {
"default": 2.0,
5.0: 3.0,
10.0: 4.0,
}
Expand All @@ -148,16 +143,13 @@ def test_build_multi_dispense_settings() -> None:
assert multi_dispense_properties.position_reference.value == "well-bottom"
assert multi_dispense_properties.offset == Coordinate(x=0, y=0, z=-5)
assert multi_dispense_properties.flow_rate_by_volume.as_dict() == {
"default": 50.0,
10.0: 40.0,
20.0: 30.0,
}
assert multi_dispense_properties.conditioning_by_volume.as_dict() == {
"default": 10.0,
5.0: 5.0,
}
assert multi_dispense_properties.disposal_by_volume.as_dict() == {
"default": 2.0,
5.0: 3.0,
}
assert multi_dispense_properties.delay.enabled is True
Expand All @@ -174,22 +166,20 @@ def test_build_multi_dispense_settings_none(

def test_liquid_handling_property_by_volume() -> None:
"""It should create a class that can interpolate values and add and delete new points."""
subject = LiquidHandlingPropertyByVolume({"default": 42, "5": 50, "10.0": 250})
assert subject.as_dict() == {"default": 42, 5.0: 50, 10.0: 250}
assert subject.default == 42.0
subject = LiquidHandlingPropertyByVolume([(5.0, 50.0), (10.0, 250.0)])
assert subject.as_dict() == {5.0: 50, 10.0: 250}
assert subject.get_for_volume(7) == 130.0

subject.set_for_volume(volume=7, value=175.5)
assert subject.as_dict() == {
"default": 42,
5.0: 50,
10.0: 250,
7.0: 175.5,
}
assert subject.get_for_volume(7) == 175.5

subject.delete_for_volume(7)
assert subject.as_dict() == {"default": 42, 5.0: 50, 10.0: 250}
assert subject.as_dict() == {5.0: 50, 10.0: 250}
assert subject.get_for_volume(7) == 130.0

with pytest.raises(KeyError, match="No value set for volume"):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_liquid_class_creation_and_property_fetching(
assert (
water.get_for(
pipette_load_name, tiprack.load_name
).dispense.flow_rate_by_volume.default
).dispense.flow_rate_by_volume.get_for_volume(1)
== 50
)
assert (
Expand Down
Loading
Loading