-
Notifications
You must be signed in to change notification settings - Fork 187
refactor(step-generation, shared-data): use set_stored_labware_items and fill_items #20293
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
base: edge
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## edge #20293 +/- ##
==========================================
- Coverage 25.59% 25.58% -0.01%
==========================================
Files 3636 3641 +5
Lines 303087 303341 +254
Branches 42340 42355 +15
==========================================
+ Hits 77560 77603 +43
- Misses 225503 225714 +211
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
TamarZanzouri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! had a few questions but this is good to merge once we get those questions answered :-)
| version=1, | ||
| count=1)`.trimStart() | ||
| flex_stacker_1 = protocol.set_stored_labware_list( | ||
| labware=[well_plate_4], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity: Is the PD UI not going to ask the user for a string so that the user has something to look at when the ODD prompts them to fill the hopper? (The message arg to fill_items().)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooooh this is an option, yes. i'll add the message arg
| }, | ||
| ], | ||
| python: 'wellPlate_1 = mock_flex_stacker_1.retrieve()', | ||
| python: 'mock_flex_stacker_1.retrieve()', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
|
||
| export const PAPI_VERSION = '2.27' // latest version from api/src/opentrons/protocols/api_support/definitions.py from the RS release branch | ||
| export const PD_APPLICATION_VERSION = '8.7.0' // latest PD version to insert into DESIGNER_APPLICATION blob | ||
| export const PAPI_VERSION = '2.28' // latest version from api/src/opentrons/protocols/api_support/definitions.py from the RS release branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, we should really change this to the oldest API version that supports all the code that PD generates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, i thought i needed to update it for the _items() but i can revert it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for this particular PD release, let's bump it up to API 2.28, because @jbleon95 added a new feature to make LC transfers more efficient that's enabled in 2.28.
But I meant that we should update the comment to say pick the oldest API version that has the features we need, rather than the latest API version.
| let parentName: string | ||
| let locationArg: string | undefined | ||
| if (onAdapter) { | ||
| if (onAdapter && !isLabwareOnHopper) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? Under what condition would something be onAdapter and isLabwareOnHopper?
And if something is on an adaptor and in the hopper, wouldn't you still want its parent to be the adaptor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya i need to fix the onAdapter name. it actually means, onLabware either an adapter or a stack of labwares.
closes EXEC-2102
Overview
Emit
fill_items()andset_stored_labware_items()instead offill()andset_stored_labware(). you can't smoke test this yet thoughAlso i reverted
retrieve()back to before lol.This change came from the meeting today:
Example code we should get from this:
Test Plan and Hands on Testing
review the code. i didn't wire up
fill_items()state update - i'll do that later on when we have things more wired up.you can actually test emitting
set_stored_labware_items()though!Changelog
add the missing types
refactor fill and set stored labware utils
add test cases
Risk assessment
low