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(step-generation: move_labware python command #17641

Merged
merged 8 commits into from
Mar 5, 2025
Merged

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Mar 4, 2025

closes AUTH-1104

Overview

This PR creates the python command for move_labware

Test Plan and Hands on Testing

There is unit test coverage for each option, feel free to smoke test it as well though!

Options include:

  1. moving to slot
  2. moving to adapter
  3. moving to module
  4. moving to waste chute
  5. moving to 4th column slot
  6. moving to trash bin (note: this is not supported in the UI at the moment, so there is no way to smoke test this!)

Changelog

  • add python command generation to moveLabware.ts
  • add test coverage for each test case
  • move getCutoutIdByAddressableArea to step-generation utils

Risk assessment

low, behind ff

@jerader jerader requested a review from a team as a code owner March 4, 2025 13:30
@jerader jerader requested a review from ddcc4 March 4, 2025 13:30
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 90.38462% with 10 lines in your changes missing coverage. Please review.

Project coverage is 25.76%. Comparing base (62d1b6e) to head (9102dd3).
Report is 6 commits behind head on edge.

Files with missing lines Patch % Lines
step-generation/src/utils/misc.ts 82.35% 6 Missing ⚠️
...neration/src/commandCreators/atomic/moveLabware.ts 94.28% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17641      +/-   ##
==========================================
+ Coverage   25.69%   25.76%   +0.07%     
==========================================
  Files        2851     2851              
  Lines      219479   219659     +180     
  Branches    17972    18043      +71     
==========================================
+ Hits        56385    56591     +206     
+ Misses     163079   163053      -26     
  Partials       15       15              
Flag Coverage Δ
protocol-designer 18.98% <26.92%> (+0.03%) ⬆️
step-generation 4.44% <90.38%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tion/utils/getAdditionalEquipmentLocationUpdate.ts 1.92% <ø> (ø)
protocol-designer/src/step-forms/utils/index.ts 77.99% <ø> (ø)
protocol-designer/src/steplist/substepTimeline.ts 33.43% <ø> (ø)
protocol-designer/src/utils/index.ts 43.71% <ø> (-5.64%) ⬇️
...neration/src/commandCreators/atomic/moveLabware.ts 88.52% <94.28%> (+2.23%) ⬆️
step-generation/src/utils/misc.ts 87.45% <82.35%> (-0.30%) ⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

},
])
expect(getSuccessResult(result).python).toBe(
`protocol.move_labware(mockPythonName, A4, use_gripper=False)`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? Does A4 need to be quoted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops, yes. good catch!

console.error('expected to find a python new location but could not')
}

const pythonUseGripper = useGripper ? 'True' : 'False'
Copy link
Contributor

Choose a reason for hiding this comment

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

The API says use_gripper defaults to False. Can we leave it out unless it's true?

},
])
expect(getSuccessResult(result).python).toBe(
`protocol.move_labware(mockPythonName, ${OFF_DECK}, use_gripper=False)`
Copy link
Contributor

Choose a reason for hiding this comment

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

For the output, let's put in the computed value of OFF_DECK so I can see if the generated code is correct.

@jerader jerader merged commit 77e93c2 into edge Mar 5, 2025
28 of 36 checks passed
@jerader jerader deleted the pd_move-labware-python branch March 5, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants