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): Correct some well geometries #17581

Draft
wants to merge 9 commits into
base: edge
Choose a base branch
from

Conversation

SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented Feb 24, 2025

Overview

Fix some errors in unreleased labware definitions that I noticed while adding tests.

Test Plan and Hands on Testing

Just automated tests.

Changelog

  • The adapter + NEST well plate combo definitions opentrons_96_aluminumblock_nest_wellplate_100ul and opentrons_96_pcr_adapter_nest_wellplate_100ul_pcr_full_skirt:

    • Fix "topHeight": 0.2, "bottomHeight": 9.89—the values need to be swapped.

      After swapping, the values match what's in the "LLD GEOMETRY LIBRARY" sheet, and what's in the standalone definition nest_96_wellplate_100ul_pcr_full_skirt.

  • opentrons_24_aluminumblock_nest_2ml_snapcap:

    • Fix a tiny gap between frustums. The corrected value comes from the "LLD GEOMETRY LIBRARY" sheet.
    • Also fix what appear to be some transcription errors: an "89" vs. "98" typo, and a missing section. The new values come from the "LLD GEOMETRY LIBRARY" sheet and match the sibling definition opentrons_24_tuberack_nest_2ml_snapcap.

And add tests to catch these kinds of errors.

Review requests

Any reason I shouldn't be swiping measurements from the "LLD GEOMETRY LIBRARY" sheet right now?

Risk assessment

Low.

Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 23.28%. Comparing base (16d00ac) to head (2d7ec2e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17581      +/-   ##
==========================================
- Coverage   25.65%   23.28%   -2.37%     
==========================================
  Files        2840     2840              
  Lines      218568   218563       -5     
  Branches    17942    17944       +2     
==========================================
- Hits        56067    50891    -5176     
- Misses     162486   167659    +5173     
+ Partials       15       13       -2     
Flag Coverage Δ
app 0.36% <ø> (-3.06%) ⬇️
protocol-designer 18.86% <ø> (ø)
shared-data 74.34% <ø> (+0.01%) ⬆️
step-generation 4.37% <ø> (ø)

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

see 101 files with indirect coverage changes

@SyntaxColoring SyntaxColoring changed the title fix(shared-data): Fix frustum stackups fix(shared-data): Correct some well geometries Feb 24, 2025
@SyntaxColoring
Copy link
Contributor Author

SyntaxColoring commented Feb 25, 2025

Tests on this PR will fail until #17580 is merged.

@SyntaxColoring SyntaxColoring marked this pull request as ready for review February 25, 2025 00:01
@SyntaxColoring SyntaxColoring requested review from a team as code owners February 25, 2025 00:01
@SyntaxColoring SyntaxColoring requested review from TamarZanzouri and a team and removed request for a team February 25, 2025 00:01
@SyntaxColoring SyntaxColoring marked this pull request as draft February 25, 2025 16:38
@SyntaxColoring
Copy link
Contributor Author

Re-drafting while I double-check the other definitions too.

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