Skip to content

Commit fe29ce4

Browse files
sanni-tjbleon95
andauthored
feat(shared-data, api): remove 'default' from all '..ByVolume' properties (#16878)
Modifies the schema from ticket AUTH-832, removes `default` key and refactors `byVolume` property schema --------- Co-authored-by: jbleon95 <[email protected]>
1 parent f37ef0a commit fe29ce4

File tree

13 files changed

+733
-676
lines changed

13 files changed

+733
-676
lines changed

api/src/opentrons/protocol_api/_liquid_properties.py

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from dataclasses import dataclass
22
from numpy import interp
3-
from typing import Optional, Dict, Sequence, Union, Tuple
3+
from typing import Optional, Dict, Sequence, Tuple
44

55
from opentrons_shared_data.liquid_classes.liquid_class_definition import (
66
AspirateProperties as SharedDataAspirateProperties,
@@ -23,31 +23,27 @@
2323

2424

2525
class LiquidHandlingPropertyByVolume:
26-
def __init__(self, properties_by_volume: Dict[str, float]) -> None:
27-
self._default = properties_by_volume["default"]
26+
def __init__(self, by_volume_property: Sequence[Tuple[float, float]]) -> None:
2827
self._properties_by_volume: Dict[float, float] = {
29-
float(volume): value
30-
for volume, value in properties_by_volume.items()
31-
if volume != "default"
28+
float(volume): value for volume, value in by_volume_property
3229
}
3330
# Volumes need to be sorted for proper interpolation of non-defined volumes, and the
3431
# corresponding values need to be in the same order for them to be interpolated correctly
3532
self._sorted_volumes: Tuple[float, ...] = ()
3633
self._sorted_values: Tuple[float, ...] = ()
3734
self._sort_volume_and_values()
3835

39-
@property
40-
def default(self) -> float:
41-
"""Get the default value not associated with any volume for this property."""
42-
return self._default
43-
44-
def as_dict(self) -> Dict[Union[float, str], float]:
36+
def as_dict(self) -> Dict[float, float]:
4537
"""Get a dictionary representation of all set volumes and values along with the default."""
46-
return self._properties_by_volume | {"default": self._default}
38+
return self._properties_by_volume
4739

4840
def get_for_volume(self, volume: float) -> float:
4941
"""Get a value by volume for this property. Volumes not defined will be interpolated between set volumes."""
5042
validated_volume = validation.ensure_positive_float(volume)
43+
if len(self._properties_by_volume) == 0:
44+
raise ValueError(
45+
"No properties found for any volumes. Cannot interpolate for the given volume."
46+
)
5147
try:
5248
return self._properties_by_volume[validated_volume]
5349
except KeyError:
@@ -66,9 +62,9 @@ def delete_for_volume(self, volume: float) -> None:
6662
"""Remove an existing volume and value from the property."""
6763
try:
6864
del self._properties_by_volume[volume]
69-
self._sort_volume_and_values()
7065
except KeyError:
7166
raise KeyError(f"No value set for volume {volume} uL")
67+
self._sort_volume_and_values()
7268

7369
def _sort_volume_and_values(self) -> None:
7470
"""Sort volume in increasing order along with corresponding values in matching order."""

api/tests/opentrons/conftest.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -804,10 +804,10 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
804804
namespace="test-fixture-2",
805805
byPipette=[
806806
ByPipetteSetting(
807-
pipetteModel="p20_single_gen2",
807+
pipetteModel="flex_1channel_50",
808808
byTipType=[
809809
ByTipTypeSetting(
810-
tiprack="opentrons_96_tiprack_20ul",
810+
tiprack="opentrons_flex_96_tiprack_50ul",
811811
aspirate=AspirateProperties(
812812
submerge=Submerge(
813813
positionReference=PositionReference.LIQUID_MENISCUS,
@@ -821,13 +821,13 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
821821
positionReference=PositionReference.WELL_TOP,
822822
offset=Coordinate(x=0, y=0, z=5),
823823
speed=100,
824-
airGapByVolume={"default": 2, "5": 3, "10": 4},
824+
airGapByVolume=[(5.0, 3.0), (10.0, 4.0)],
825825
touchTip=TouchTipProperties(enable=False),
826826
delay=DelayProperties(enable=False),
827827
),
828828
positionReference=PositionReference.WELL_BOTTOM,
829829
offset=Coordinate(x=0, y=0, z=-5),
830-
flowRateByVolume={"default": 50, "10": 40, "20": 30},
830+
flowRateByVolume=[(10.0, 40.0), (20.0, 30.0)],
831831
preWet=True,
832832
mix=MixProperties(enable=False),
833833
delay=DelayProperties(
@@ -845,16 +845,16 @@ def minimal_liquid_class_def2() -> LiquidClassSchemaV1:
845845
positionReference=PositionReference.WELL_TOP,
846846
offset=Coordinate(x=0, y=0, z=5),
847847
speed=100,
848-
airGapByVolume={"default": 2, "5": 3, "10": 4},
848+
airGapByVolume=[(5.0, 3.0), (10.0, 4.0)],
849849
blowout=BlowoutProperties(enable=False),
850850
touchTip=TouchTipProperties(enable=False),
851851
delay=DelayProperties(enable=False),
852852
),
853853
positionReference=PositionReference.WELL_BOTTOM,
854854
offset=Coordinate(x=0, y=0, z=-5),
855-
flowRateByVolume={"default": 50, "10": 40, "20": 30},
855+
flowRateByVolume=[(10.0, 40.0), (20.0, 30.0)],
856856
mix=MixProperties(enable=False),
857-
pushOutByVolume={"default": 5, "10": 7, "20": 10},
857+
pushOutByVolume=[(10.0, 7.0), (20.0, 10.0)],
858858
delay=DelayProperties(enable=False),
859859
),
860860
multiDispense=None,

api/tests/opentrons/protocol_api/test_liquid_class.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ def test_get_for_pipette_and_tip(
2121
) -> None:
2222
"""It should get the properties for the specified pipette and tip."""
2323
liq_class = LiquidClass.create(minimal_liquid_class_def2)
24-
result = liq_class.get_for("p20_single_gen2", "opentrons_96_tiprack_20ul")
24+
result = liq_class.get_for("flex_1channel_50", "opentrons_flex_96_tiprack_50ul")
2525
assert result.aspirate.flow_rate_by_volume.as_dict() == {
26-
"default": 50.0,
2726
10.0: 40.0,
2827
20.0: 30.0,
2928
}
@@ -36,7 +35,7 @@ def test_get_for_raises_for_incorrect_pipette_or_tip(
3635
liq_class = LiquidClass.create(minimal_liquid_class_def2)
3736

3837
with pytest.raises(ValueError):
39-
liq_class.get_for("p20_single_gen2", "no_such_tiprack")
38+
liq_class.get_for("flex_1channel_50", "no_such_tiprack")
4039

4140
with pytest.raises(ValueError):
42-
liq_class.get_for("p300_single", "opentrons_96_tiprack_20ul")
41+
liq_class.get_for("no_such_pipette", "opentrons_flex_96_tiprack_50ul")

api/tests/opentrons/protocol_api/test_liquid_class_properties.py

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

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

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

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

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

@@ -75,7 +74,6 @@ def test_build_single_dispense_settings() -> None:
7574
assert single_dispense_properties.retract.offset == Coordinate(x=0, y=0, z=5)
7675
assert single_dispense_properties.retract.speed == 100
7776
assert single_dispense_properties.retract.air_gap_by_volume.as_dict() == {
78-
"default": 2.0,
7977
5.0: 3.0,
8078
10.0: 4.0,
8179
}
@@ -93,15 +91,13 @@ def test_build_single_dispense_settings() -> None:
9391
assert single_dispense_properties.position_reference.value == "well-bottom"
9492
assert single_dispense_properties.offset == Coordinate(x=0, y=0, z=-5)
9593
assert single_dispense_properties.flow_rate_by_volume.as_dict() == {
96-
"default": 50.0,
9794
10.0: 40.0,
9895
20.0: 30.0,
9996
}
10097
assert single_dispense_properties.mix.enabled is True
10198
assert single_dispense_properties.mix.repetitions == 3
10299
assert single_dispense_properties.mix.volume == 15
103100
assert single_dispense_properties.push_out_by_volume.as_dict() == {
104-
"default": 5.0,
105101
10.0: 7.0,
106102
20.0: 10.0,
107103
}
@@ -111,7 +107,7 @@ def test_build_single_dispense_settings() -> None:
111107

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

@@ -131,7 +127,6 @@ def test_build_multi_dispense_settings() -> None:
131127
assert multi_dispense_properties.retract.offset == Coordinate(x=0, y=0, z=5)
132128
assert multi_dispense_properties.retract.speed == 100
133129
assert multi_dispense_properties.retract.air_gap_by_volume.as_dict() == {
134-
"default": 2.0,
135130
5.0: 3.0,
136131
10.0: 4.0,
137132
}
@@ -148,16 +143,13 @@ def test_build_multi_dispense_settings() -> None:
148143
assert multi_dispense_properties.position_reference.value == "well-bottom"
149144
assert multi_dispense_properties.offset == Coordinate(x=0, y=0, z=-5)
150145
assert multi_dispense_properties.flow_rate_by_volume.as_dict() == {
151-
"default": 50.0,
152146
10.0: 40.0,
153147
20.0: 30.0,
154148
}
155149
assert multi_dispense_properties.conditioning_by_volume.as_dict() == {
156-
"default": 10.0,
157150
5.0: 5.0,
158151
}
159152
assert multi_dispense_properties.disposal_by_volume.as_dict() == {
160-
"default": 2.0,
161153
5.0: 3.0,
162154
}
163155
assert multi_dispense_properties.delay.enabled is True
@@ -174,22 +166,20 @@ def test_build_multi_dispense_settings_none(
174166

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

182173
subject.set_for_volume(volume=7, value=175.5)
183174
assert subject.as_dict() == {
184-
"default": 42,
185175
5.0: 50,
186176
10.0: 250,
187177
7.0: 175.5,
188178
}
189179
assert subject.get_for_volume(7) == 175.5
190180

191181
subject.delete_for_volume(7)
192-
assert subject.as_dict() == {"default": 42, 5.0: 50, 10.0: 250}
182+
assert subject.as_dict() == {5.0: 50, 10.0: 250}
193183
assert subject.get_for_volume(7) == 130.0
194184

195185
with pytest.raises(KeyError, match="No value set for volume"):

api/tests/opentrons/protocol_api_integration/test_liquid_classes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def test_liquid_class_creation_and_property_fetching(
3232
assert (
3333
water.get_for(
3434
pipette_load_name, tiprack.load_name
35-
).dispense.flow_rate_by_volume.default
35+
).dispense.flow_rate_by_volume.get_for_volume(1)
3636
== 50
3737
)
3838
assert (

0 commit comments

Comments
 (0)