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

feat(shared-data): labware schema v3 #16027

Merged
merged 17 commits into from
Aug 26, 2024
Merged

Conversation

caila-marashaj
Copy link
Contributor

@caila-marashaj caila-marashaj commented Aug 15, 2024

Overview

For static liquid tracking to convert between inner well heights and volumes, we need to have access to the inner dimensions of wells.

This adds a new labware schema to include inner well geometry, as well as typescript bindings for the new labware definition.

Changelog

  • add labware schema v3
  • add typescript bindings for labware definition 3
  • fix radiusOfCurvature variable name from python binding
  • add a couple fixtures and ts tests for new schema

TODO

Once we add actual labware definitions, we should add some python binding tests in test_validations.py

@caila-marashaj caila-marashaj force-pushed the EXEC-647-labware-schema3 branch from 6a86c11 to 6bd53bf Compare August 19, 2024 18:06
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Thank you!

As discussed in-person, we'll focus on the JSON Schema side of this first, and then update the Python and TS bindings when the dust has settled.

I think the JSON Schema needs some restructuring to work as intended. One way to test it is to feed it into a tool like this and click around.

shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
@caila-marashaj caila-marashaj marked this pull request as ready for review August 20, 2024 19:54
@caila-marashaj caila-marashaj requested review from a team as code owners August 20, 2024 19:54
@caila-marashaj caila-marashaj requested review from smb2268 and removed request for a team August 20, 2024 19:54
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Yep, the JSON schema looks mostly right now, thanks! Just a few touchups:

shared-data/labware/schemas/3.json Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
shared-data/labware/schemas/3.json Outdated Show resolved Hide resolved
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

And comments for the TypeScript part, when you get there.

Same deal. Looks basically right, just needing a few small touchups.

shared-data/js/types.ts Outdated Show resolved Hide resolved
shared-data/js/types.ts Outdated Show resolved Hide resolved
shared-data/js/types.ts Outdated Show resolved Hide resolved
shared-data/js/types.ts Show resolved Hide resolved
@caila-marashaj caila-marashaj force-pushed the EXEC-647-labware-schema3 branch from 75fa041 to 610aae6 Compare August 26, 2024 15:58
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

LGTM but I think linting is failing.

shared-data/js/types.ts Outdated Show resolved Hide resolved
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.

Looking good to me pending the inline comments

shared-data/python/tests/labware/__init__.py Show resolved Hide resolved
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

Yay!

@caila-marashaj caila-marashaj merged commit a1a00d9 into edge Aug 26, 2024
51 checks passed
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.

3 participants