refactor(app, labware-designer, labware-library, opentrons-ai-client, share-data): Move all labware images to shared-data #19088

rclarke0 · 2025-07-30T20:11:20Z

Overview

Relocate labware images from opentrons-ai-client, labware-designer, app, and labware-library to shared-data

Test Plan and Hands on Testing

Tested labware-library, tested the app, tested labware-creator

Changelog

Removed duplicate labware-images.ts and images folder in the app, opentrons-ai-client, labware-designer, and labware-library
Kept an images folder in labware-designer that only contains svgs of labware shapes
in shared-data, a labware-images.ts can be run to generate labware-images-generated.ts . This automatically pairs the labware load name with the associated image based on the file name. This will pair labware + the adapter if part of the load name string is found in the image file name.
in 'shared-data' added Pillow library to pip file for use in format_image.py in order to crop and resize image before uploading it to shared-data folder

Review requests

confirm that this is the desired format

Risk assessment

Are there any other spots referencing the now deleted images file?

codecov · 2025-07-30T20:12:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.77%. Comparing base (c2c764a) to head (0f0b925).
⚠️ Report is 23 commits behind head on edge.

Additional details and impacted files

@@             Coverage Diff             @@
##             edge   #19088       +/-   ##
===========================================
+ Coverage   23.84%   54.77%   +30.93%     
===========================================
  Files        3373     3373               
  Lines      296660   296979      +319     
  Branches    31486    37800     +6314     
===========================================
+ Hits        70736   162684    +91948     
+ Misses     225901   134030    -91871     
- Partials       23      265      +242

Flag	Coverage Δ
protocol-designer	`19.02% <96.34%> (+0.08%)`	⬆️
step-generation	`5.42% <96.34%> (+0.04%)`	⬆️

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

Files with missing lines	Coverage Δ
app/src/local-resources/labware/utils/index.ts	`100.00% <ø> (+100.00%)`	⬆️
...rc/molecules/WizardRequiredEquipmentList/index.tsx	`80.88% <100.00%> (+80.88%)`	⬆️
...CalibrateTipLength/AskForCalibrationBlockModal.tsx	`82.82% <100.00%> (+82.82%)`	⬆️
.../Desktop/CalibrationPanels/ChosenTipRackRender.tsx	`74.28% <100.00%> (+74.28%)`	⬆️
...ganisms/Desktop/Labware/LabwareDetails/Gallery.tsx	`71.05% <ø> (+71.05%)`	⬆️
...ware-library/src/components/labware-ui/Gallery.tsx	`0.00% <ø> (ø)`
shared-data/js/labware.ts	`96.05% <100.00%> (+4.27%)`	⬆️

... and 1741 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

shared-data/labware/images/image_details/labware-images.ts

…for missing images

SyntaxColoring

Thank you, this seems like a huge improvement! Glad the glob import thing is working out so far.

Changes requested for a couple things, which I'll elaborate on below:

Some images going missing (probably unintentional) and some images changing (OK if it's intentional, but not sure).
Possible delineation of a "primary image," for the one (?) case where that matters.

Besides that, just small, optional, code-level nitpicks.

We should also get input from someone else who's better-versed in our JS machinery, since this is largely new to me.

SyntaxColoring · 2025-08-07T16:18:00Z

opentrons-ai-client/src/assets/images/labwares/favicon.ico

Just double-checking that it's okay to remove this favicon.ico file, and the other two favicon.ico files? I can't see where they were being used, if they even were being used, so it seems fine? 🤷

shared-data/tsconfig-data.json

SyntaxColoring · 2025-08-07T16:37:16Z

shared-data/js/__tests__/labwareImages.test.ts

+])
+
+const definitionModules = import.meta.glob(
+  '../../labware/definitions/2/*/*.{json,ts}',


Roughly the same comment as in labware-images.ts:

Instead of writing out a glob import for the labware definitions, can we use one of the existing functions or constants in labware.ts?

if I use functions in labware.ts it creates a circular dependency since labware.ts needs labwareImages. I could move all of this into labware.ts?

SyntaxColoring · 2025-08-07T16:40:38Z

shared-data/Pipfile

@@ -33,3 +33,4 @@ numpy = "==1.22.3"
 # transitive dependency. 3.22 introduces a build plugin called coherent.licensed that
 # is not safe to have installed when another project is being built.
 zipp = "==3.21.0"
+pillow = "*"


I don't think it actually matters in this case, but for clarity, can we move this to [dev-packages]? That might need a regeneration of Pipfile.lock, not sure.

SyntaxColoring · 2025-08-07T17:17:28Z

app/src/molecules/WizardRequiredEquipmentList/index.tsx

-    imageSrc = labwareImages[loadName as keyof typeof labwareImages]
+    imageSrc = labwareImages[loadName as keyof typeof labwareImages][0]


Do we need some way to convey which image is "primary"? Or at least make sure the images are sorted in a deliberate way such that [0] is always "primary"?

You can imagine, like, a tip rack having a picture of the whole tip rack, and a separate picture of just a single tip. The prior behavior of this code was to show the whole tip rack, and we probably want to keep doing that. As-is, I don't think anything guarantees that we wouldn't accidentally show the single-tip picture.

It creates the list in alphabetical order so I don't think we need to

fixed this to sort labwareImages

SyntaxColoring · 2025-08-07T20:03:43Z

shared-data/labware/images/image_details/labware-images.ts

+      let i = 0
+      for (let j = 0; j < varParts.length; j++) {
+        if (loadParts[i] === varParts[j]) {
+          i++
+        }
+        if (i === loadParts.length) break
+      }


Is this just checking to see if loadParts is the same as varParts?

If so, we can use isEqual from Lodash (import isEqual from "lodash/isEqual").

Yes, it's shocking to me too that JavaScript doesn't have a built-in way to do this. :)

Here, I want to check if loadParts is within varParts in order to map tube racks and tube definitions so we don't have as many duplicate image cases.

SyntaxColoring · 2025-08-07T20:17:39Z

shared-data/labware/images/image_details/labware-images.ts

+  if (matchingUrls.length > 0) {
+    labwareImages[loadName] = matchingUrls
+  }


As written, if a labware doesn't have any images, it looks like this will result in labwareImages["labware_load_name"] being undefined.

But when calling code does labwareImages["labware_load_name"], TypeScript will always think the type of images is string[]. So there's a hazard of type errors, e.g. if someone did labwareImages["labware_load_name"].map(...) without remembering to check for the undefined case.

So, could we either:

Change labwareImages: Record<string, string[]> to Record<string, string[] | undefined>, making sure callers can't forget to handle the undefined case.

Or, change this code to set labwareImages[loadName] to [], so there is no undefined case.

I would probably do this one.

I might have seen you added a test somewhere in here to make sure that each labware always has at least one image, so this might not be a problem in practice. Just a safety thing in case there end up being any exceptions where that doesn't hold true, that's easy to add now.

SyntaxColoring · 2025-08-07T20:17:52Z

shared-data/labware/images/image_details/labware-images.ts

+// 6. Add unmatched images using their file name as loadName
+for (const [varName, url] of Object.entries(imageKeyToUrl)) {
+  if (!matchedImageVars.has(varName)) {
+    labwareImages[varName] = [url]
+  }
+}


No real action needed, just thinking aloud.

To use one of these unmatched images, calling code is presumably going to have to do labwareImages["some_hard_coded_key"][0]. That's fine, but if they can do that—if they know what the hard-coded key is—couldn't they have just directly imported the image file in the first place, instead of going through this labware-images.ts aggregation logic?

I'm a little bit fuzzy on the right way to accomplish that, which is why I'm not outright suggesting it. Like, there are definitely ways for other projects to directly reach into shared-data's file tree and import individual files, and we definitely have precedent for that, but I'm not sure if it's "right"—maybe the "right" way is for those images to be in shared-data's package.json's exports, for example. Maybe someone better-versed in JS packaging can answer.

The broader context behind my thinking is that there's some evidence that we've been overusing "barrel files," i.e. files that just re-export other things. They can cause circular dependencies, increase test run times, maybe increase bundle sizes, and maybe interfere with hot module reloading. So it might be beneficial to figure out a direct import pattern, instead of encouraging callers to go through labware-images.ts.

There's not really consensus on this yet, though.

SyntaxColoring · 2025-08-07T20:33:23Z

shared-data/labware/images/image_details/labware-images.ts

+  const normalizedLoadName = loadName.replace(/\./g, '_').replace(/-/g, '_')
+  const loadParts = normalizedLoadName.split('_')
+
+  const matchingUrls = Object.entries(imageKeyToUrl)


Structural suggestion for readability. Could we have a helper function whose signature is something like:

function doesImageBelongToLabware(loadName: string, imagePath: string): boolean { ... }

And then we'd centralize all the path parsing and normalization in there. As-is, it's a little difficult to follow because some of the normalization happens in here while other parts of it happen up above, and they need to stay in sync with each other.

SyntaxColoring · 2025-08-07T20:59:03Z

shared-data/labware/images/image_details/labware-images.ts

+    })
+
+  if (matchingUrls.length > 0) {
+    labwareImages[loadName] = matchingUrls


I suspect we'll want some kind of sort here to make sure the URLs for each labware are always in the same order. When iterating over files in the filesystem, it's generally good practice to not assume they'll be in any particular order. (It can depend on the OS, filesystem, locale settings, ...)

rclarke0 added 3 commits July 30, 2025 13:34

creating new location for labware images

22f2e2c

move images into shared-data

58cb304

removing duplicate svgs

2239c7d

rclarke0 requested review from a team as code owners July 30, 2025 20:11

rclarke0 added DO NOT REVIEW YET draft labels Jul 30, 2025

rclarke0 requested review from smb2268 and removed request for a team July 30, 2025 20:11

rclarke0 and others added 8 commits July 30, 2025 16:25

fix lint errors

7cae0a1

revert deleted file

60a88ae

pipfile

3a1a358

Merge branch 'edge' into relocate-labware-images

2e5004e

fix lint error in abr-testing folder

90dacf2

fix prettier errors

660495e

prettier fix 2

61ff951

prettier fix

0708c8f

rclarke0 removed DO NOT REVIEW YET draft labels Jul 31, 2025

rclarke0 added 2 commits July 31, 2025 21:43

add readme file

824c774

removing additional labwareimages.ts

aa08e71

rclarke0 marked this pull request as draft August 1, 2025 01:59

rclarke0 added 4 commits July 31, 2025 22:34

add test

f7b5f52

lint fix

afe9921

lint fixes

24048ba

fix lint

a647008

SyntaxColoring reviewed Aug 1, 2025

View reviewed changes

shared-data/labware/images/image_details/labware-images.ts Outdated Show resolved Hide resolved

rclarke0 added 2 commits August 1, 2025 12:37

fixes ot2 calibration bus

53f96fa

changed labware-images.ts to use glob and updated test case to check …

1b41de8

…for missing images

rclarke0 and others added 10 commits August 4, 2025 10:29

prettier

f5d6d20

updating images

88bd96a

draft images

0821c00

changing some images

0865e73

labware images

dff9107

Merge branch 'edge' into relocate-labware-images

a1fc4b8

fix test

7722b82

eppendorf labware

7588294

change to local import statement

01e8547

prettier fixes on README.md

6f9c6b4

rclarke0 marked this pull request as ready for review August 7, 2025 13:19

rclarke0 requested a review from SyntaxColoring August 7, 2025 13:20

SyntaxColoring requested changes Aug 7, 2025

View reviewed changes

combined labware-images.ts with labware.ts and resized images

0f0b925

rclarke0 requested a review from SyntaxColoring August 8, 2025 14:34

		imageSrc = labwareImages[loadName as keyof typeof labwareImages]
		imageSrc = labwareImages[loadName as keyof typeof labwareImages][0]

refactor(app, labware-designer, labware-library, opentrons-ai-client, share-data): Move all labware images to shared-data #19088

Are you sure you want to change the base?

refactor(app, labware-designer, labware-library, opentrons-ai-client, share-data): Move all labware images to shared-data #19088

Uh oh!

Conversation

rclarke0 commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Test Plan and Hands on Testing

Changelog

Review requests

Risk assessment

Uh oh!

codecov bot commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

SyntaxColoring left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rclarke0 commented Jul 30, 2025 •

edited

Loading

codecov bot commented Jul 30, 2025 •

edited

Loading