-
Notifications
You must be signed in to change notification settings - Fork 179
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(api,robot-server,shared-data): Split LabwareDefinition
Pydantic model for labware schemas 2 and 3
#17563
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
88102d0
Refactor TypedDict well definitions and fix geometryDefinitionId.
SyntaxColoring 6ca0311
Split parameters.
SyntaxColoring 1b02df9
Reorder keys to make changes more obvious.
SyntaxColoring ab25726
Split well shapes.
SyntaxColoring 72ec525
Split root LabwareDefinition model.
SyntaxColoring 1bc8391
shared-data updates.
SyntaxColoring d2dd586
api updates.
SyntaxColoring 9c1f0e4
robot-server updates.
SyntaxColoring 87596c7
Merge branch 'edge' into split_pydantic_labwaredefinition
SyntaxColoring File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up for this temporary hack that I'm doing for OT-2 tip length calibration.
The ticket I linked, EXEC-1230, currently talks about totally blocking these calibration flows from dealing with labware schema 3. I'm changing my mind on that—I think we have to get them to deal with labware schema 3 correctly somehow—and I'll rewrite EXEC-1230 when I know more about the approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you think we have to get them to deal with labware schema 3 correctly? We could instead not ever issue OT-2 tiprack or calibration block definitions in schema 3. We possibly lock out loading tipracks in schema 3 when the robot is an OT-2, for this reason. This means that OT-2 tipracks wouldn't get the benefit of the more-correct geometry loading, but this is an ancient and known problem with OT-2 tipracks that people have been handing for years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically because I didn't feel it was acceptable to abandon OT-2 tip racks like that. If our goal is to improve labware positioning, but we fail at standard OT-2 tip racks, then that's a pretty big bummer. They're certainly not edge cases. I think it's worth trying to resolve even if it means us (or me) pushing a little harder.
Locking OT-2 tip racks and calibration blocks to schema 2 sounds like a good escape hatch, if it comes down to it, but I hope it doesn't come down to it.
I can deprioritize this and revisit the question later, but mark it as something we must resolve somehow before v8.4.0—either get it to work with schema 3, or delete the schema 3 defs and lock it to schema 2.