Skip to content

feat(api): add aspirate_flow_rate/dispense_flow_rate to mix()#18405

Merged
ddcc4 merged 2 commits intoedgefrom
dc-api-mixrate
May 28, 2025
Merged

feat(api): add aspirate_flow_rate/dispense_flow_rate to mix()#18405
ddcc4 merged 2 commits intoedgefrom
dc-api-mixrate

Conversation

@ddcc4
Copy link
Copy Markdown
Contributor

@ddcc4 ddcc4 commented May 20, 2025

Overview

Add the aspirate_flow_rate and dispense_flow_rate absolute flow arguments to the PAPI mix() function. AUTH-1624

This follows the theme of adding absolute flow rate support to the InstrumentContext functions that previously took a rate ratio.

PD allows you to independently specify the flow rate for the aspirate and dispense in a Mix step, but the PAPI mix() function previously only allowed a single rate argument, which is a ratio that is multiplied by the pipette's default aspirate and dispense flow rates. (PD step generation worked around this limitation by changing the pipette's default aspirate/dispense rates for every Mix step. This change will let us generate simpler code.)

Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of aspirate_flow_rate and dispense_flow_rate.

Risk assessment

Medium. We're changing the public API.

@ddcc4 ddcc4 requested review from jbleon95 and jerader May 20, 2025 22:35
@ddcc4 ddcc4 requested a review from a team as a code owner May 20, 2025 22:35
rate,
# We have to hide all new arguments from protocol_api_old/test_context.py.
# I don't know if the test is even valid, but I'm afraid to change the test.
**{"flow_rate": aspirate_flow_rate}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rather us remove an old test that might have questionable value than using hacks in our production code to work around it. What is the problematic test in question?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What is the problematic test in question?

It's protocol_api_old/test_context.py, like mentioned in the comment. It has a number of problems:

  • The mock functions in that test (fake_aspirate() and fake_dispense()) have totally different argument names from what's in our current API (e.g., fake_aspirate() takes vol and loc, whereas our API takes volume and location). That means we can't even call aspirate() and dispense() with named arguments, because it would break the test.
  • The mock functions in that test also has different default arguments from what we have in the API (e.g., fake_aspirate() takes rate=None, whereas our API takes rate=1). That makes it hard to write code that works the same with both our API and that test.
  • The expected calls in the test are very specific, which makes it hard to add new arguments to our API.

So ... I think the test is annoying, but I don't know the history of the test and why it's there, so I'm afraid to just change the test, because it might be enforcing some behavior we need to preserve for ancient versions of the robot stack or something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, I've removed the silly kwargs workarounds now that we no longer need to support protocol_api_old/test_context.py.

@ddcc4 ddcc4 force-pushed the dc-api-mixrate branch from 599a59e to 327054b Compare May 22, 2025 18:33
Copy link
Copy Markdown
Contributor

@jbleon95 jbleon95 left a comment

Choose a reason for hiding this comment

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

All looks good!

@ddcc4 ddcc4 merged commit 6f9b158 into edge May 28, 2025
24 checks passed
@ddcc4 ddcc4 deleted the dc-api-mixrate branch May 28, 2025 21:30
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
ddcc4 added a commit that referenced this pull request May 29, 2025
…18405)

# Overview

Add the `aspirate_flow_rate` and `dispense_flow_rate` absolute flow
arguments to the PAPI `mix()` function. AUTH-1624

This follows the theme of adding absolute flow rate support to the
`InstrumentContext` functions that previously took a `rate` ratio.

PD allows you to independently specify the flow rate for the aspirate
and dispense in a Mix step, but the PAPI `mix()` function previously
only allowed a single `rate` argument, which is a ratio that is
multiplied by the pipette's default aspirate and dispense flow rates.
(PD step generation worked around this limitation by changing the
pipette's default aspirate/dispense rates for every Mix step. This
change will let us generate simpler code.)

## Test Plan and Hands on Testing

Added unit tests. You should be able to set one or both or none of
`aspirate_flow_rate` and `dispense_flow_rate`.

## Risk assessment

Medium. We're changing the public API.

(cherry picked from commit 6f9b158)
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