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(api): add new InstrumentContext.transfer_liquid() method #16819

Open
wants to merge 10 commits into
base: edge
Choose a base branch
from

Conversation

sanni-t
Copy link
Member

@sanni-t sanni-t commented Nov 14, 2024

Closes AUTH-843

Overview

Adds InstrumentContext.transfer_liquid() method that does the following-

  • validates parameters of transfer_liquid()
  • loads the liquid class properties for the relevant pipette and tiprack into protocol engine
  • delegates to engine core to perform the actual transfer

This PR does not cover engine core's transfer method execution.

Test Plan and Hands on Testing

Since this is mostly adding the scaffolding to implement the actual execution of transfer method, this PR is not testable on the robot yet. Unit tests are crucial at this stage and have been added for all changes.

Changelog

  • adds InstrumentContext.transfer_liquid()
  • adds InstrumentCore.load_liquid_class() and a placeholder InstrumentCore.transfer_liquid()
  • adds validator methods and their tests to protocol_api.validation that get used in InstrumentContext.transfer_liquid()
  • adds as_schema_v1_model() to the liquid properties in order to fetch the pydantic model representation of the liquid class's properties. This is used by load_liquid_class() in order to create a liquidClassRecord
  • adds tests

Review requests

Do the validations make sense? Am I missing anything?
Does the transfer_liquid() interface look good?

Risk assessment

No risk so far since this is a code-only change.

Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.17%. Comparing base (d5b7e61) to head (b551782).
Report is 19 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #16819      +/-   ##
==========================================
+ Coverage   73.90%   79.17%   +5.26%     
==========================================
  Files          43      120      +77     
  Lines        3231     4514    +1283     
==========================================
+ Hits         2388     3574    +1186     
- Misses        843      940      +97     
Flag Coverage Δ
g-code-testing 92.43% <ø> (?)

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

see 77 files with indirect coverage changes

@sanni-t sanni-t marked this pull request as ready for review November 25, 2024 23:04
@sanni-t sanni-t requested a review from a team as a code owner November 25, 2024 23:04
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.

1 participant