Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 0 deletions api/src/opentrons/protocol_api/_liquid_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ def get_for_volume(self, volume: float) -> float:
interp(validated_volume, self._sorted_volumes, self._sorted_values)
)

def set_for_all_volumes(self, value: float) -> None:
"""Override all existing volume-dependent values with the given value."""
self.clear_values()
self.set_for_volume(0, value)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, would it make sense to call your new function set_for_all_volumes(), as a nice contrast to the existing function set_for_volume()?


def set_for_volume(self, volume: float, value: float) -> None:
"""Add a new volume and value for the property for the interpolation curve."""
validated_volume = validation.ensure_positive_float(volume)
Expand Down
16 changes: 16 additions & 0 deletions api/tests/opentrons/protocol_api/test_liquid_class_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,22 @@ def test_liquid_handling_property_by_volume() -> None:
assert subject.get_for_volume(1) == 50.0
assert subject.get_for_volume(1000) == 250.0

# Test fixed overrides
subject.set_for_all_volumes(4321)
for volume in (0, 1, 7, 100):
assert subject.get_for_volume(volume) == 4321

# Test resetting to default
subject.reset_values()
assert subject.as_dict() == {5.0: 50, 10.0: 250}
assert subject.get_for_volume(7) == 130.0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're testing for volume=7 here, it would be nice if the test above (line 459) also tested for volume=7, to demonstrate how the value changes as you call the API functions.


# Test clearing all values
subject.clear_values()
assert subject.as_dict() == {}
with pytest.raises(ValueError, match="No properties found for any volumes"):
subject.get_for_volume(7)


def test_non_existent_property_raises_error() -> None:
"""It should raise an attribute error if the set property does not exist."""
Expand Down