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): Remove obsolete "_gen3" pipette names #13514

Merged
merged 16 commits into from
Sep 15, 2023
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
22 changes: 22 additions & 0 deletions api-client/src/instruments/__fixtures__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,25 @@ export const instrumentsResponseFixture = {
},
],
}

export const instrumentsResponseLeftPipetteFixture = {
data: { channels: 1, min_volume: 1, max_volume: 50 },
instrumentModel: 'p1000_single_v3.0',
instrumentName: 'p1000_single_flex',
instrumentType: 'pipette',
mount: 'left',
serialNumber: 'abc',
subsystem: 'pipette_left',
ok: true,
}

export const instrumentsResponseRightPipetteFixture = {
data: { channels: 1, min_volume: 1, max_volume: 50 },
instrumentModel: 'p1000_single_v3.0',
instrumentName: 'p1000_single_flex',
instrumentType: 'pipette',
mount: 'right',
serialNumber: 'cba',
subsystem: 'pipette_right',
ok: true,
}
Comment on lines +30 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are test fixtures for the /instruments endpoints, but they were in the directory for the /pipettes endpoints. I'm moving them here and correcting their instrumentName from p1000_single_gen3 to p1000_single_flex.

22 changes: 0 additions & 22 deletions api-client/src/pipettes/__fixtures__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,25 +252,3 @@ export const pipetteSettingsResponseFixture = {
},
},
}

export const pipetteDataLeftFixture = {
data: { channels: 1, min_volume: 1, max_volume: 50 },
instrumentModel: 'p1000_single_v3.0',
instrumentName: 'p1000_single_gen3',
instrumentType: 'pipette',
mount: 'left',
serialNumber: 'abc',
subsystem: 'pipette_left',
ok: true,
}

export const pipetteResponseRightFixture = {
data: { channels: 1, min_volume: 1, max_volume: 50 },
instrumentModel: 'p1000_single_v3.0',
instrumentName: 'p1000_single_gen3',
instrumentType: 'pipette',
mount: 'right',
serialNumber: 'cba',
subsystem: 'pipette_right',
ok: true,
}
46 changes: 23 additions & 23 deletions api/src/opentrons/protocol_api/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,26 @@
# The first APIVersion where Python protocols can specify deck labels like "D1" instead of "1".
_COORDINATE_DECK_LABEL_VERSION_GATE = APIVersion(2, 15)

# Mapping of user-facing pipette names to names used by the internal Opentrons system
_FLEX_PIPETTE_NAMES_MAP = {
"p50_single_gen3": "p50_single_flex",
"flex_1channel_50": "p50_single_flex",
"p50_multi_gen3": "p50_multi_flex",
"flex_8channel_50": "p50_multi_flex",
"p1000_single_gen3": "p1000_single_flex",
"flex_1channel_1000": "p1000_single_flex",
"p1000_multi_gen3": "p1000_multi_flex",
"flex_8channel_1000": "p1000_multi_flex",
"flex_96channel_1000": "p1000_96",
# Mapping of public Python Protocol API pipette load names
# to names used by the internal Opentrons system
_PIPETTE_NAMES_MAP = {
"p10_single": PipetteNameType.P10_SINGLE,
"p10_multi": PipetteNameType.P10_MULTI,
"p20_single_gen2": PipetteNameType.P20_SINGLE_GEN2,
"p20_multi_gen2": PipetteNameType.P20_MULTI_GEN2,
"p50_single": PipetteNameType.P50_SINGLE,
"p50_multi": PipetteNameType.P50_MULTI,
"p300_single": PipetteNameType.P300_SINGLE,
"p300_multi": PipetteNameType.P300_MULTI,
"p300_single_gen2": PipetteNameType.P300_SINGLE_GEN2,
"p300_multi_gen2": PipetteNameType.P300_MULTI_GEN2,
"p1000_single": PipetteNameType.P1000_SINGLE,
"p1000_single_gen2": PipetteNameType.P1000_SINGLE_GEN2,
"flex_1channel_50": PipetteNameType.P50_SINGLE_FLEX,
"flex_8channel_50": PipetteNameType.P50_MULTI_FLEX,
"flex_1channel_1000": PipetteNameType.P1000_SINGLE_FLEX,
"flex_8channel_1000": PipetteNameType.P1000_MULTI_FLEX,
"flex_96channel_1000": PipetteNameType.P1000_96,
}


Expand Down Expand Up @@ -109,20 +118,11 @@ def ensure_pipette_name(pipette_name: str) -> PipetteNameType:
pipette_name = ensure_lowercase_name(pipette_name)

try:
if pipette_name in _FLEX_PIPETTE_NAMES_MAP.keys():
# TODO (spp: 2023-07-11): !!! VERY IMPORTANT!!!
# We DO NOT want to support the old 'gen3' suffixed names for Flex launch.
# This provision to accept the old names is added only for maintaining
# backwards compatibility during internal testing and should be phased out.
# So remove this name mapping and conversion at an appropriate time before launch
checked_name = PipetteNameType(_FLEX_PIPETTE_NAMES_MAP[pipette_name])
else:
checked_name = PipetteNameType(pipette_name)
return checked_name
except ValueError as e:
return _PIPETTE_NAMES_MAP[pipette_name]
except KeyError:
raise ValueError(
f"Cannot resolve {pipette_name} to pipette, must be given valid pipette name."
) from e
) from None


def ensure_and_convert_deck_slot(
Expand Down
4 changes: 2 additions & 2 deletions api/src/opentrons/protocol_engine/execution/gantry_mover.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
# We rely on this being the same for every OT-3 pipette.
#
# We found this number by peeking at the height that OT3Simulator returns for these pipettes:
# * Single- and 8-Channel P50 GEN3
# * Single-, 8-, and 96-channel P1000 GEN3
# * Flex Single- and 8-Channel P50
# * Flex Single-, 8-, and 96-channel P1000
#
# That OT3Simulator return value is what Protocol Engine uses for simulation when Protocol Engine
# is configured to not virtualize pipettes, so this number should match it.
Expand Down
4 changes: 1 addition & 3 deletions api/tests/opentrons/hardware_control/test_instruments.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def dummy_instruments_attached_ot3():
types.Mount.LEFT: {
"model": "p1000_single_v3.3",
"id": "testy",
"name": "p1000_single_gen3",
"name": "flex_1channel_1000",
},
types.Mount.RIGHT: {"model": None, "id": None, "name": None},
OT3Mount.GRIPPER: None,
Expand Down Expand Up @@ -680,12 +680,10 @@ async def test_reset_instruments(monkeypatch, sim_and_instr):
types.Mount.LEFT: {
"model": "p1000_single_v3.3",
"id": "testy",
"name": "p1000_single_gen3",
},
types.Mount.RIGHT: {
"model": "p1000_single_v3.3",
"id": "testy",
"name": "p1000_single_gen3",
},
}
sim_builder, _ = sim_and_instr
Expand Down
41 changes: 33 additions & 8 deletions api/tests/opentrons/protocol_api/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,27 @@ def test_ensure_mount_input_invalid() -> None:
@pytest.mark.parametrize(
["input_value", "expected"],
[
# Every OT-2 pipette:
("p10_single", PipetteNameType.P10_SINGLE),
("p10_multi", PipetteNameType.P10_MULTI),
("p50_single", PipetteNameType.P50_SINGLE),
("p50_multi", PipetteNameType.P50_MULTI),
("p300_single", PipetteNameType.P300_SINGLE),
("P300_muLTI_gen2", PipetteNameType.P300_MULTI_GEN2),
(
"p50_single_gen3",
PipetteNameType.P50_SINGLE_FLEX,
), # Remove this line once we phase out '_gen3' names
("p300_multi", PipetteNameType.P300_MULTI),
("p1000_single", PipetteNameType.P1000_SINGLE),
("p20_single_gen2", PipetteNameType.P20_SINGLE_GEN2),
("p20_multi_gen2", PipetteNameType.P20_MULTI_GEN2),
("p300_single_gen2", PipetteNameType.P300_SINGLE_GEN2),
("p300_multi_gen2", PipetteNameType.P300_MULTI_GEN2),
("p1000_single_gen2", PipetteNameType.P1000_SINGLE_GEN2),
# Every Flex pipette:
("flex_1channel_50", PipetteNameType.P50_SINGLE_FLEX),
("flex_8channel_50", PipetteNameType.P50_MULTI_FLEX),
("flex_1channel_1000", PipetteNameType.P1000_SINGLE_FLEX),
("flex_8channel_1000", PipetteNameType.P1000_MULTI_FLEX),
("flex_96channel_1000", PipetteNameType.P1000_96),
# Weird capitalization:
("P300_muLTI_gen2", PipetteNameType.P300_MULTI_GEN2),
],
)
def test_ensure_pipette_name(input_value: str, expected: PipetteNameType) -> None:
Expand All @@ -81,10 +94,22 @@ def test_ensure_pipette_name(input_value: str, expected: PipetteNameType) -> Non
assert result == expected


def test_ensure_pipette_input_invalid() -> None:
@pytest.mark.parametrize(
"input_value",
[
"oh-no", # Not even remotely a pipette name.
"p1000_single_gen3", # Obsolete name for Flex pipette.
"p1000_single_flex", # Internal-only name for Flex pipette.
"p1000_96", # Internal-only name for Flex pipette.
],
)
def test_ensure_pipette_input_invalid(input_value: str) -> None:
"""It should raise a ValueError if given an invalid name."""
with pytest.raises(ValueError, match="must be given valid pipette name"):
subject.ensure_pipette_name("oh-no")
with pytest.raises(
ValueError,
match=f"Cannot resolve {input_value} to pipette, must be given valid pipette name",
):
subject.ensure_pipette_name(input_value)


@pytest.mark.parametrize(
Expand Down
2 changes: 1 addition & 1 deletion api/tests/opentrons/protocol_api_old/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ def test_instrument_trash_ot3(ctx, get_labware_def):
ctx.home()

mount = Mount.LEFT
instr = ctx.load_instrument("p1000_single_gen3", mount)
instr = ctx.load_instrument("flex_1channel_1000", mount)

assert instr.trash_container.name == "opentrons_1_trash_3200ml_fixed"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ def run(protocol: protocol_api.ProtocolContext):
EPM = reagent_plate.wells_by_name()["A7"]

# pipette
p1000 = protocol.load_instrument("p1000_multi_gen3", "left", tip_racks=[tiprack_200_1, tiprack_200_2])
p50 = protocol.load_instrument("p50_multi_gen3", "right", tip_racks=[tiprack_20])
p1000 = protocol.load_instrument("flex_8channel_1000", "left", tip_racks=[tiprack_200_1, tiprack_200_2])
p50 = protocol.load_instrument("flex_8channel_50", "right", tip_racks=[tiprack_20])

# tip and sample tracking
sample_well = "A3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ def run(protocol: protocol_api.ProtocolContext):

# pipette
p1000 = protocol.load_instrument(
"p1000_multi_gen3", "left", tip_racks=[tiprack_200_1, tiprack_200_2, tiprack_200_3]
"flex_8channel_1000", "left", tip_racks=[tiprack_200_1, tiprack_200_2, tiprack_200_3]
)
p50 = protocol.load_instrument("p50_multi_gen3", "right", tip_racks=[tiprack_50_1, tiprack_50_2])
p50 = protocol.load_instrument("flex_8channel_50", "right", tip_racks=[tiprack_50_1, tiprack_50_2])

# tip and sample tracking
if COLUMNS == 1:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ def run(protocol: protocol_api.ProtocolContext):
Barcodes3 = reagent_plate.wells_by_name()["A9"]

# pipette
p1000 = protocol.load_instrument("p1000_multi_gen3", "right", tip_racks=[tiprack_200_1, tiprack_200_2])
p50 = protocol.load_instrument("p50_multi_gen3", "left", tip_racks=[tiprack_20])
p1000 = protocol.load_instrument("flex_8channel_1000", "right", tip_racks=[tiprack_200_1, tiprack_200_2])
p50 = protocol.load_instrument("flex_8channel_50", "left", tip_racks=[tiprack_20])

# tip and sample tracking
if COLUMNS == 3:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def run(protocol: protocol_api.ProtocolContext):

# pipette
p1000 = protocol.load_instrument(
"p1000_single_gen3",
"flex_1channel_1000",
"right",
tip_racks=[tiprack_200_1, tiprack_200_2, tiprack_200_3, tiprack_200_4, tiprack_200_5, tiprack_200_6],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def run(protocol: protocol_api.ProtocolContext):

# pipette
if USE_8xMULTI == "YES":
p1000 = protocol.load_instrument("p1000_multi_gen3", "right")
p1000 = protocol.load_instrument("flex_1channel_1000", "right")
else:
p1000 = protocol.load_instrument("p1000_96", "left")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ def run(protocol: protocol_api.ProtocolContext):
DIL = reservoir["A5"]

# pipette
p50 = protocol.load_instrument("p50_multi_gen3", "left", tip_racks=[tiprack_50_1, tiprack_50_2])
p1000 = protocol.load_instrument("p1000_multi_gen3", "right", tip_racks=[tiprack_200_1])
p50 = protocol.load_instrument("flex_8channel_50", "left", tip_racks=[tiprack_50_1, tiprack_50_2])
p1000 = protocol.load_instrument("flex_8channel_1000", "right", tip_racks=[tiprack_200_1])

# samples
src_file_path = inspect.getfile(lambda: None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import * as React from 'react'
import { renderHook } from '@testing-library/react-hooks'
import { useInstrumentsQuery } from '@opentrons/react-api-client'
import {
pipetteDataLeftFixture,
pipetteResponseRightFixture,
instrumentsResponseLeftPipetteFixture,
instrumentsResponseRightPipetteFixture,
} from '@opentrons/api-client'
import { useAttachedPipettesFromInstrumentsQuery } from '..'

Expand All @@ -17,7 +17,10 @@ describe('useAttachedPipettesFromInstrumentsQuery hook', () => {
it('returns attached pipettes', () => {
mockUseInstrumentsQuery.mockReturnValue({
data: {
data: [pipetteDataLeftFixture, pipetteResponseRightFixture],
data: [
instrumentsResponseLeftPipetteFixture,
instrumentsResponseRightPipetteFixture,
],
},
} as any)

Expand All @@ -30,11 +33,11 @@ describe('useAttachedPipettesFromInstrumentsQuery hook', () => {

expect(result.current).toEqual({
left: {
...pipetteDataLeftFixture,
...instrumentsResponseLeftPipetteFixture,
displayName: 'Flex 1-Channel 1000 μL',
},
right: {
...pipetteResponseRightFixture,
...instrumentsResponseRightPipetteFixture,
displayName: 'Flex 1-Channel 1000 μL',
},
})
Expand Down
7 changes: 1 addition & 6 deletions hardware-testing/hardware_testing/gravimetric/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,7 @@ def _get_volumes(ctx: ProtocolContext, cfg: config.VolumetricConfig) -> List[flo
def _load_pipette(
ctx: ProtocolContext, cfg: config.VolumetricConfig
) -> InstrumentContext:
load_str_channels = {1: "single_gen3", 8: "multi_gen3", 96: "96"}
pip_channels = cfg.pipette_channels
if pip_channels not in load_str_channels:
raise ValueError(f"unexpected number of channels: {pip_channels}")
chnl_str = load_str_channels[pip_channels]
pip_name = f"p{cfg.pipette_volume}_{chnl_str}"
pip_name = f"flex_{cfg.pipette_channels}channel_{cfg.pipette_volume}"
ui.print_info(f'pipette "{pip_name}" on mount "{cfg.pipette_mount}"')

# if we're doing multiple tests in one run, the pipette may already be loaded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_multi_gen3", "left")
pipette = ctx.load_instrument("flex_8channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def run(ctx: ProtocolContext) -> None:
for slot in slots
]
vial = ctx.load_labware(LABWARE_ON_SCALE, SLOT_SCALE)
pipette = ctx.load_instrument("p1000_single_gen3", "left")
pipette = ctx.load_instrument("flex_1channel_1000", "left")
for rack in tipracks:
pipette.pick_up_tip(rack["A1"])
pipette.aspirate(10, vial["A1"].top())
Expand Down
Loading