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

fix(shared-data): Keep inner well geometries of sibling labware in sync with each other #17606

Merged
merged 5 commits into from
Mar 3, 2025

Conversation

SyntaxColoring
Copy link
Contributor

Overview

This fixes some out-of-date labware geometry definitions and tries to prevent similar problems in the future.

Some of our labware definitions ought to share well geometries with each other. For example, opentrons_24_aluminumblock_nest_0.5ml_screwcap and opentrons_24_tuberack_nest_0.5ml_screwcap have the same NEST 0.5 mL screwcap tubes.

It's proving to be challenging to keep all of these in sync. The underlying spreadsheets from the hardware team don't necessarily enumerate every single definition. So, some labware seem like they were accidentally omitted from the updates in #17082, and other labware just never had any geometry definitions to begin with.

This PR fixes the discrepancies that I found. Then, to mitigate the problem going forward, this PR hard-codes list of labware that belong to the same "shared geometry groups." In the tests, we assert that each group has consistent geometry.

Test Plan and Hands on Testing

None.

Changelog

  • Hard-code lists of labware that should share well geometry with each other. Test that they actually do.
  • Fix several discrepancies caught by those tests. See the commit messages for exact details.

Review requests

  • Am I reconciling each group in the correct direction?
  • Are any labware groups missing?
  • Are all the labware groups correct (i.e. they actually share the same physical tubes)?

Risk assessment

Medium. This might reconcile a group in the wrong direction.

@SyntaxColoring SyntaxColoring requested review from a team as code owners February 27, 2025 18:33
@SyntaxColoring SyntaxColoring requested review from mjhuff and removed request for a team February 27, 2025 18:33
This copies geometry across definitions:

opentrons_24_tuberack_generic_2ml_screwcap
--> opentrons_24_aluminumblock_generic_2ml_screwcap.

The tuberack one was already correct before this PR, but the aluminumblock one didn't have any geometry, even though it's the same tubes.
This copies geometry across definitions:

opentrons_24_tuberack_nest_1.5ml_snapcap
--> opentrons_24_aluminumblock_nest_1.5ml_snapcap

The tuberack one was already correct before this PR, but its updates in PR #17082 were never propagated to the aluminumblock one.
This copies geometry across definitions:

opentrons_24_tuberack_nest_2ml_snapcap
--> opentrons_24_aluminumblock_nest_2ml_snapcap.

The tuberack one was already correct before this PR, but its updates in PR #17082 were never propagated to the aluminumblock one.
This copies geometry across definitions:

opentrons_10_tuberack_nest_4x50ml_6x15ml_conical
--> opentrons_6_tuberack_nest_50ml_conical
--> opentrons_15_tuberack_nest_15ml_conical

4x50ml_6x15ml was already correct before this PR, but its updates in #17082 were never propagated to the other two definitions.
Copy link

codecov bot commented Feb 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.07%. Comparing base (6afc259) to head (04f64bf).
Report is 16 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17606      +/-   ##
==========================================
- Coverage   63.08%   63.07%   -0.01%     
==========================================
  Files        2840     2843       +3     
  Lines      218766   219410     +644     
  Branches    18142    18323     +181     
==========================================
+ Hits       138010   138400     +390     
- Misses      80564    80818     +254     
  Partials      192      192              
Flag Coverage Δ
protocol-designer 19.01% <ø> (+0.14%) ⬆️
step-generation 4.38% <ø> (+<0.01%) ⬆️

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

see 26 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Yeah, this is a good fix. Thanks!

@SyntaxColoring SyntaxColoring merged commit 062e2b3 into edge Mar 3, 2025
72 checks passed
@SyntaxColoring SyntaxColoring deleted the keep_siblings_in_sync branch March 3, 2025 15:24
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