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

fix(api): Error handling for unsupported pipette configuration behavior with partial column and row #15961

Merged
merged 10 commits into from
Aug 14, 2024
61 changes: 41 additions & 20 deletions api/src/opentrons/protocol_api/instrument_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -2077,6 +2077,8 @@ def configure_nozzle_layout( # noqa: C901
f"Nozzle layout configuration of style {style.value} is unsupported in API Versions lower than {_PARTIAL_NOZZLE_CONFIGURATION_SINGLE_ROW_PARTIAL_COLUMN_ADDED_IN}."
)

front_right_resolved = front_right
back_left_resolved = back_left
if style != NozzleLayout.ALL:
if start is None:
raise ValueError(
Expand All @@ -2086,30 +2088,49 @@ def configure_nozzle_layout( # noqa: C901
raise ValueError(
f"Starting nozzle specified is not one of {types.ALLOWED_PRIMARY_NOZZLES}"
)
if style == NozzleLayout.QUADRANT:
if front_right is None and back_left is None:
raise ValueError(
"Cannot configure a QUADRANT layout without a front right or back left nozzle."
)
elif not (front_right is None and back_left is None):
raise ValueError(
f"Parameters 'front_right' and 'back_left' cannot be used with {style.value} Nozzle Configuration Layout."
)
if style == NozzleLayout.ROW:
Copy link
Member

Choose a reason for hiding this comment

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

Should we repeat the same check for COLUMN style too?

if self.channels != 96:
raise ValueError(
"Row configuration is only supported on 96-Channel pipettes."
)
if style == NozzleLayout.PARTIAL_COLUMN:
if self.channels == 1 or self.channels == 96:
raise ValueError(
"Partial column configuration is only supported on 8-Channel pipettes."
)

front_right_resolved = front_right
back_left_resolved = back_left
if style == NozzleLayout.PARTIAL_COLUMN:
if end is None:
if end is None:
raise ValueError(
"Partial column configurations require the 'end' parameter."
)
if start[0] in end:
raise ValueError(
"The 'start' and 'end' parameters of a partial column configuration cannot be in the same row."
)
# Determine if 'end' will be configured as front_right or back_left
if start == "H1" or start == "H12":
if "A" in end:
raise ValueError(
f"A partial column configuration with 'start'={start} cannot have its 'end' parameter be in row A."
)
back_left_resolved = end
elif start == "A1" or start == "A12":
if "H" in end:
raise ValueError(
f"A partial column configuration with 'start'={start} cannot have its 'end' parameter be in row H."
)
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should explain that a column configured to use nozzles A through H does not constitute a 'partial' column and that the user should use an ALL configuration instead. (Can you configure an 8-channel with a COLUMN configuration?)

front_right_resolved = end

if style == NozzleLayout.QUADRANT:
if front_right is None and back_left is None:
raise ValueError(
"Cannot configure a QUADRANT layout without a front right or back left nozzle."
)
elif not (front_right is None and back_left is None):
raise ValueError(
"Parameter 'end' is required for Partial Column Nozzle Configuration Layout."
f"Parameters 'front_right' and 'back_left' cannot be used with a {style.value} nozzle configuration."
)

# Determine if 'end' will be configured as front_right or back_left
if start == "H1" or start == "H12":
back_left_resolved = end
elif start == "A1" or start == "A12":
front_right_resolved = end

self._core.configure_nozzle_layout(
style,
primary_nozzle=start,
Expand Down
36 changes: 36 additions & 0 deletions api/tests/opentrons/protocol_api/test_instrument_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,7 @@ def test_prepare_to_aspirate_checks_volume(
[NozzleLayout.ROW, "E1", None, None, pytest.raises(ValueError)],
[NozzleLayout.PARTIAL_COLUMN, "H1", None, "G1", does_not_raise()],
[NozzleLayout.PARTIAL_COLUMN, "H1", "H1", "G1", pytest.raises(ValueError)],
[NozzleLayout.PARTIAL_COLUMN, "H1", None, "A1", pytest.raises(ValueError)],
],
)
def test_configure_nozzle_layout(
Expand All @@ -1152,6 +1153,41 @@ def test_configure_nozzle_layout(
)


@pytest.mark.parametrize(
argnames=[
"pipette_channels",
"style",
"primary_nozzle",
"front_right_nozzle",
"end",
"exception",
],
argvalues=[
[8, NozzleLayout.PARTIAL_COLUMN, "A1", None, "G1", does_not_raise()],
[96, NozzleLayout.PARTIAL_COLUMN, "H1", None, "G1", pytest.raises(ValueError)],
[8, NozzleLayout.ROW, "H1", None, None, pytest.raises(ValueError)],
[96, NozzleLayout.ROW, "H1", None, None, does_not_raise()],
],
)
def test_pipette_supports_nozzle_layout(
subject: InstrumentContext,
decoy: Decoy,
mock_instrument_core: InstrumentCore,
pipette_channels: int,
style: NozzleLayout,
primary_nozzle: Optional[str],
front_right_nozzle: Optional[str],
end: Optional[str],
exception: ContextManager[None],
) -> None:
"""Test that error is raised when a pipette attempts to use an unsupported layout."""
decoy.when(mock_instrument_core.get_channels()).then_return(pipette_channels)
with exception:
subject.configure_nozzle_layout(
style=style, start=primary_nozzle, end=end, front_right=front_right_nozzle
)


@pytest.mark.parametrize("api_version", [APIVersion(2, 15)])
def test_dispense_0_volume_means_dispense_everything(
decoy: Decoy,
Expand Down
Loading