refactor(app): clean up invariantContext construct for PV #20168
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #20168 +/- ##
===========================================
- Coverage 56.85% 24.29% -32.57%
===========================================
Files 3572 3573 +1
Lines 297865 297999 +134
Branches 42200 41988 -212
===========================================
- Hits 169363 72387 -96976
- Misses 128272 225590 +97318
+ Partials 230 22 -208
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ncdiehl11
left a comment
ncdiehl11
left a comment
There was a problem hiding this comment.
left a few minor style comments
step-generation/src/utils/constructInvariantContextFromAnalysis.ts
Outdated
Show resolved
Hide resolved
step-generation/src/utils/constructInvariantContextFromAnalysis.ts
Outdated
Show resolved
Hide resolved
step-generation/src/utils/constructInvariantContextFromAnalysis.ts
Outdated
Show resolved
Hide resolved
step-generation/src/utils/constructInvariantContextFromAnalysis.ts
Outdated
Show resolved
Hide resolved
step-generation/src/utils/constructInvariantContextFromAnalysis.ts
Outdated
Show resolved
Hide resolved
| const otherEntities = commands.reduce( | ||
| ( | ||
| acc: Omit< | ||
| InvariantContext, | ||
| 'labwareEntities' | 'moduleEntities' | 'pipetteEntities' | ||
| >, | ||
| command: RunTimeCommand | ||
| ) => { |
There was a problem hiding this comment.
| const otherEntities = commands.reduce( | |
| ( | |
| acc: Omit< | |
| InvariantContext, | |
| 'labwareEntities' | 'moduleEntities' | 'pipetteEntities' | |
| >, | |
| command: RunTimeCommand | |
| ) => { | |
| const otherEntities = commands.reduce< | |
| Omit< | |
| InvariantContext, | |
| 'labwareEntities' | 'moduleEntities' | 'pipetteEntities' | |
| > | |
| >( | |
| (acc, command: RunTimeCommand) => { |
There was a problem hiding this comment.
this is the opposite of the other suggestions lol. i'm gonna leave as is
There was a problem hiding this comment.
Hm what do you mean? Typing the return of the reduce rather than the acc is consistent above, no?
Fine with me to leave as is though
There was a problem hiding this comment.
ooooo i definitely didn't read clearly. my bad. ya i'll address this! thanks!
| if (addressableAreaName.includes('WasteChute')) { | ||
| wasteChuteEntities = { | ||
| [id]: { | ||
| pythonName: 'waste_chute', | ||
| id, | ||
| location, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
don't we have the same if statement above?
There was a problem hiding this comment.
nah the other one is for trash bin
ncdiehl11
left a comment
ncdiehl11
left a comment
There was a problem hiding this comment.
just replied to one comment re: what I think is a redundant if statement, but will unblock you! thanks a lot
closes AUTH-2486
Overview
clean up the invariant context construct for PV utility by grabbing the analysis and using the completed protocol analysis
labware,modules, andpipettes. and then the staging areas, waste chute and trash bins are all generated based off of the commands stillalso fixed a few bugs with generating the trash and waste chute entities and pipette entities with multiple tipracks assigned
Test Plan and Hands on Testing
review the code and smoke test that all the deck setup information is properly rendering on the deck map
Changelog
refactor the util and rename to say its from the analysis instead of just commands
Risk assessment
low