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(protocol-designer): load module in python file #17483

Merged
merged 8 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from 7 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
20 changes: 14 additions & 6 deletions protocol-designer/src/file-data/__tests__/createFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ describe('createFile selector', () => {
afterEach(() => {
vi.restoreAllMocks()
})
const entities = {
moduleEntities: v7Fixture.moduleEntities,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

turns out the v7Fixture doesn't have a module - should i add one to the fixture so we can test that it works in the unit test?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to! But like the comment below says, the point of the tests in this file is just to make sure createFile()/createPythonFile() spit out something that looks like a protocol. We'll have other unit tests in other files to make sure that the generated Python code is actually correct for all possible types of modules/labware/etc.

labwareEntities,
pipetteEntities,
liquidEntities: ingredients,
}
it('should return a schema-valid JSON V8 protocol', () => {
// @ts-expect-error(sa, 2021-6-15): resultFunc not part of Selector type
const result = createFile.resultFunc(
Expand All @@ -78,16 +84,13 @@ describe('createFile selector', () => {
v7Fixture.robotStateTimeline,
OT2_ROBOT_TYPE,
dismissedWarnings,
ingredients,
ingredLocations,
v7Fixture.savedStepForms,
v7Fixture.orderedStepIds,
labwareEntities,
v7Fixture.moduleEntities,
pipetteEntities,
labwareNicknamesById,
labwareDefsByURI,
{}
{},
entities
)
expectResultToMatchSchema(result)

Expand All @@ -99,7 +102,12 @@ describe('createFile selector', () => {

it('should return a valid Python protocol file', () => {
// @ts-expect-error(sa, 2021-6-15): resultFunc not part of Selector type
const result = createPythonFile.resultFunc(fileMetadata, OT2_ROBOT_TYPE, {})
const result = createPythonFile.resultFunc(
fileMetadata,
OT2_ROBOT_TYPE,
entities,
v7Fixture.initialRobotState
)
// This is just a quick smoke test to make sure createPythonFile() produces
// something that looks like a Python file. The individual sections of the
// generated Python will be tested in separate unit tests.
Expand Down
57 changes: 55 additions & 2 deletions protocol-designer/src/file-data/__tests__/pythonFile.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
import { describe, it, expect } from 'vitest'
import { FLEX_ROBOT_TYPE, OT2_ROBOT_TYPE } from '@opentrons/shared-data'
import { pythonMetadata, pythonRequirements } from '../selectors/pythonFile'
import {
FLEX_ROBOT_TYPE,
HEATERSHAKER_MODULE_TYPE,
HEATERSHAKER_MODULE_V1,
MAGNETIC_BLOCK_TYPE,
MAGNETIC_BLOCK_V1,
OT2_ROBOT_TYPE,
} from '@opentrons/shared-data'
import {
getLoadModules,
pythonMetadata,
pythonRequirements,
} from '../selectors/pythonFile'
import type { TimelineFrame } from '@opentrons/step-generation'
import type { ModuleEntities } from '../../step-forms'

describe('pythonMetadata', () => {
it('should generate metadata section', () => {
Expand Down Expand Up @@ -50,3 +63,43 @@ requirements = {
)
})
})

describe('getLoadModules', () => {
it('should generate loadModules', () => {
const moduleId = '1'
const moduleId2 = '2'
const moduleId3 = '3'
const mockModuleEntities: ModuleEntities = {
[moduleId]: {
id: moduleId,
model: MAGNETIC_BLOCK_V1,
type: MAGNETIC_BLOCK_TYPE,
pythonName: 'magnetic_block_1',
},
[moduleId2]: {
id: moduleId2,
model: HEATERSHAKER_MODULE_V1,
type: HEATERSHAKER_MODULE_TYPE,
pythonName: 'heater_shaker_1',
},
[moduleId3]: {
id: moduleId3,
model: MAGNETIC_BLOCK_V1,
type: MAGNETIC_BLOCK_TYPE,
pythonName: 'magnetic_block_2',
},
}
const modules: TimelineFrame['modules'] = {
[moduleId]: { slot: 'B1', moduleState: {} as any },
[moduleId2]: { slot: 'A1', moduleState: {} as any },
[moduleId3]: { slot: 'A2', moduleState: {} as any },
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Could you get rid of the blank line at here before the code in the function?

expect(getLoadModules(mockModuleEntities, modules)).toBe(
`# Load Modules:
magnetic_block_1 = protocol.load_module("magneticBlockV1", "B1")
heater_shaker_1 = protocol.load_module("heaterShakerModuleV1", "A1")
magnetic_block_2 = protocol.load_module("magneticBlockV1", "A2")`
)
})
})
24 changes: 13 additions & 11 deletions protocol-designer/src/file-data/selectors/fileCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,34 +99,34 @@ export const createFile: Selector<ProtocolFile> = createSelector(
getRobotStateTimeline,
getRobotType,
dismissSelectors.getAllDismissedWarnings,
stepFormSelectors.getLiquidEntities,
ingredSelectors.getLiquidsByLabwareId,
stepFormSelectors.getSavedStepForms,
stepFormSelectors.getOrderedStepIds,
stepFormSelectors.getLabwareEntities,
stepFormSelectors.getModuleEntities,
stepFormSelectors.getPipetteEntities,
uiLabwareSelectors.getLabwareNicknamesById,
labwareDefSelectors.getLabwareDefsByURI,
getStepGroups,
stepFormSelectors.getInvariantContext,
(
fileMetadata,
initialRobotState,
robotStateTimeline,
robotType,
dismissedWarnings,
liquidEntities,
ingredLocations,
savedStepForms,
orderedStepIds,
labwareEntities,
moduleEntities,
pipetteEntities,
labwareNicknamesById,
labwareDefsByURI,
stepGroups
stepGroups,
invariantContext
) => {
const { author, description, created } = fileMetadata
const {
pipetteEntities,
labwareEntities,
liquidEntities,
moduleEntities,
} = invariantContext

const loadCommands = getLoadCommands(
initialRobotState,
Expand Down Expand Up @@ -308,14 +308,16 @@ export const createFile: Selector<ProtocolFile> = createSelector(
export const createPythonFile: Selector<string> = createSelector(
getFileMetadata,
getRobotType,
(fileMetadata, robotType) => {
stepFormSelectors.getInvariantContext,
getInitialRobotState,
(fileMetadata, robotType, invariantContext, robotState) => {
return (
[
// Here are the sections of the Python file:
pythonImports(),
pythonMetadata(fileMetadata),
pythonRequirements(robotType),
pythonDefRun(),
pythonDefRun(invariantContext, robotState),
]
.filter(section => section) // skip any blank sections
.join('\n\n') + '\n'
Expand Down
40 changes: 37 additions & 3 deletions protocol-designer/src/file-data/selectors/pythonFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
import { FLEX_ROBOT_TYPE, OT2_ROBOT_TYPE } from '@opentrons/shared-data'
import {
formatPyDict,
formatPyStr,
indentPyLines,
PROTOCOL_CONTEXT_NAME,
} from '@opentrons/step-generation'
import type { FileMetadataFields } from '../types'
import type {
InvariantContext,
ModuleEntities,
TimelineFrame,
} from '@opentrons/step-generation'
import type { RobotType } from '@opentrons/shared-data'
import type { FileMetadataFields } from '../types'

const PAPI_VERSION = '2.23' // latest version from api/src/opentrons/protocols/api_support/definitions.py

Expand Down Expand Up @@ -51,9 +57,37 @@ export function pythonRequirements(robotType: RobotType): string {
return `requirements = ${formatPyDict(requirements)}`
}

export function pythonDefRun(): string {
export function getLoadModules(
moduleEntities: ModuleEntities,
moduleRobotState: TimelineFrame['modules']
): string {
const hasModules = Object.keys(moduleEntities).length > 0
const pythonModules = hasModules
? Object.values(moduleEntities)
.map(module => {
// pythonIdentifier from api/src/opentrons/protocol_api/validation.py#L373
const pythonIdentifier = module.model
return `${
module.pythonName
} = ${PROTOCOL_CONTEXT_NAME}.load_module(${formatPyStr(
pythonIdentifier
Copy link
Contributor

Choose a reason for hiding this comment

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

I think just using module.model directly is fine now if we don't need the lookup function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup! fair.

)}, ${formatPyStr(moduleRobotState[module.id].slot)})`
})
.join('\n')
: ''
return hasModules ? `# Load Modules:\n${pythonModules}` : ''
}

export function pythonDefRun(
invariantContext: InvariantContext,
robotState: TimelineFrame
): string {
const { moduleEntities } = invariantContext

const loadModules = getLoadModules(moduleEntities, robotState.modules)

const sections: string[] = [
// loadModules(),
loadModules,
// loadLabware(),
// loadInstruments(),
// defineLiquids(),
Expand Down
Loading