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

refactor(shared-data): Disallow empty labware geometries #17580

Merged
merged 3 commits into from
Feb 25, 2025

Conversation

SyntaxColoring
Copy link
Contributor

Overview

In labware schema v3:

  • A labware has wells (of course).
  • A well can refer to a geometry definition, if we have detailed information about the shape of the well's interior.
  • Each geometry definition has a list of vertically-stacked sections.

Some definitions were providing a well geometry, but with an empty list of sections. This was adding an edge case to the tests and production code. This PR forbids that, so geometries either need to have real data, or they need to be omitted.

Test Plan and Hands on Testing

Just automated tests.

Changelog

  • In the JSON schema, require the sections array to have at least one element.
  • In any labware definitions that were supplying an empty geometry, just delete the geometry.
    • Four of these are for "generic" labware. By definition, it doesn't seem like we could ever have detailed geometry info for these.
      • definitions/3/opentrons_24_aluminumblock_generic_2ml_screwcap/3.json
      • definitions/3/opentrons_24_tuberack_generic_0.75ml_snapcap_acrylic/2.json
      • definitions/3/opentrons_40_aluminumblock_eppendorf_24x2ml_safelock_snapcap_generic_16x0.2ml_pcr_strip/2.json
      • definitions/3/opentrons_96_aluminumblock_generic_pcr_strip_200ul/3.json
    • The remaining one is definitions/3/opentrons_24_tuberack_eppendorf_2ml_safelock_snapcap_acrylic/2.json. This labware is very old, apparently predating the 4-in-1 tube rack set. It does not appear in Labware Library and does not have geometry measurements in the "LLD GEOMETRY LIBRARY" sheet. I think it's safe to say we're never going to care about getting good geometry info for it. I think it's questionable whether a schema 3 definition should exist for it at all, let alone one with innerLabwareGeometry.

Review requests

Does my thinking in the changelog above sound correct?

Risk assessment

Low.

@SyntaxColoring SyntaxColoring requested a review from a team February 24, 2025 22:34
@SyntaxColoring SyntaxColoring requested a review from a team as a code owner February 24, 2025 22:34
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.88%. Comparing base (6546cd4) to head (0ae9ad6).
Report is 2 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #17580      +/-   ##
==========================================
+ Coverage   25.65%   25.88%   +0.23%     
==========================================
  Files        2840     2812      -28     
  Lines      218569   216598    -1971     
  Branches    17942    17878      -64     
==========================================
- Hits        56068    56063       -5     
+ Misses     162486   160520    -1966     
  Partials       15       15              
Flag Coverage Δ
protocol-designer 18.86% <ø> (ø)
step-generation 4.37% <ø> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...pentrons_shared_data/labware/labware_definition.py 80.58% <ø> (-0.08%) ⬇️

... and 41 files with indirect coverage changes

@SyntaxColoring SyntaxColoring requested a review from a team as a code owner February 25, 2025 14:18
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, I think that makes perfect sense to me!

@SyntaxColoring SyntaxColoring merged commit 16d00ac into edge Feb 25, 2025
67 checks passed
@SyntaxColoring SyntaxColoring deleted the no_empty_geometries branch February 25, 2025 15:56
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