feat(shared-data, protocol-designer, api): move labware versions by a… #19358
Conversation
…pi level to shared-data closes AUTH-2225
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #19358 +/- ##
===========================================
+ Coverage 23.90% 54.58% +30.67%
===========================================
Files 3392 3393 +1
Lines 299732 299980 +248
Branches 32754 39266 +6512
===========================================
+ Hits 71657 163735 +92078
+ Misses 228058 135981 -92077
- Partials 17 264 +247
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Looks reasonable so far!
In Slack discussions, there are questions about the premise of all of this, and whether we should really be basing this on robot software release versions instead of apiLevels. But I think this is a good incremental improvement over, like, manually keeping LABWAREV2_DO_NOT_LIST up to date, and we should see how far we can get with this approach.
api/src/opentrons/protocol_api/core/engine/_default_labware_versions.py
Outdated
Show resolved
Hide resolved
api/src/opentrons/protocol_api/core/engine/_default_labware_versions.py
Outdated
Show resolved
Hide resolved
|
TODO:
|
| from opentrons.protocol_api.core.engine._default_labware_versions import ( | ||
| from opentrons_shared_data.labware._default_labware_versions import ( |
There was a problem hiding this comment.
_default_labware_versions was previously prefixed with an underscore because it was a private implementation detail of opentrons.protocol_api.core.engine. Since it's now a public interface of opentrons_shared_data.labware, let's remove the underscore.
|
|
||
| @functools.cache | ||
| def _parse_json_from_filesystem() -> DefaultLabwareVersions: | ||
| with open(DEFAULT_LABWARE_VERSIONS_FILE) as file: |
There was a problem hiding this comment.
I think we have to do open(get_shared_data_root() / DEFAULT_LABWARE_VERSIONS_FILE), not just open(DEFAULT_LABWARE_VERSIONS_FILE). Otherwise, the path will be relative to the process's current working directory.
| from pathlib import Path | ||
|
|
||
| from typing import TypeAlias | ||
| from opentrons.protocols.api_support.types import APIVersion |
There was a problem hiding this comment.
Ah. We can't import opentrons from opentrons_shared_data, unfortunately—that's a circular dependency.
I think your options here are either:
- Move the
get_standard_labware_default_version()processing logic back toapi/. (Sorry for misleading you! 😬) This file inshared-data/can remain, to either exposeparse_json_from_filesystem()to the file inapi/, or exposeDEFAULT_LABWARE_VERSIONS_FILEto the file inapi/. So the data would be owned byshared-data/, and maybe also the parsing; but the processing would be owned byapi/. - Or, rewrite this file to not use
APIVersion. Maybe it can just be replaced withtuple[int, int]?
No real preference from me.
| def _get_available_load_names_and_highest_versions() -> list[tuple[str, int]]: | ||
| versions_by_api = _parse_json_from_filesystem() | ||
| highest_available_version_by_load_name: dict[str, int] = {} | ||
| for load_name, version, _ in list_labware_definitions(): | ||
| if ( | ||
| load_name not in highest_available_version_by_load_name | ||
| or version > highest_available_version_by_load_name[load_name] | ||
| ): | ||
| highest_available_version_by_load_name[load_name] = version | ||
| for labware_map in versions_by_api.values(): | ||
| for load_name, version in labware_map.items(): | ||
| if ( | ||
| load_name not in highest_available_version_by_load_name | ||
| or version > highest_available_version_by_load_name[load_name] | ||
| ): | ||
| highest_available_version_by_load_name[load_name] = version | ||
| return sorted(highest_available_version_by_load_name.items()) |
There was a problem hiding this comment.
I think this is changing the behavior of _get_available_load_names_and_highest_versions() in a way that we don't want:
Formerly, it was returning the maximum version of every single labware definition in shared-data, using labware/definitions/{2,3}/*/*.json as the source of truth.
With these changes, it's returning the maximum version of every single labware mentioned in the API version map.
The difference matters because of how this function was being used in test_default_labware_version_coverage(). It's like an independent source to check the API version map against. So if we implement this in terms of the API version map, the test becomes kind of tautological.
…pi level to shared-data
closes AUTH-2225
Overview
Please read the description in the ticket. But basically, previously, PD would get the latest labware def versions that are available in shared-data. This causes issues because some versions are not supported in the Robot stack version that is public or will be released before the upcoming PD 8.6 release.
This PR first moves the list of labware that are bumped in api level 2.26 to shared-data into its own JSON file. Then, that JSOn file is imported into the api in
_default_labware_versions.pyto reference for the labware versions gated by api level 2.26.Next, the JSON file is exported for JS/TS files to use. I created a
getUnsupportedLabwareDefVersionsByApiLevel()util that gets all the labware versions that are not supported based on the api level that pd's python emits. And then I plugged it into PD'sgetOnlyLatestDefs()There is a README that explains how everything works.
Test Plan and Hands on Testing
Create a protocol in PD and add a labware that has a version bump in api level 2.26 -> for example, the bio-rad 96 well plate. Export the protocol and open it and see that the version loaded for the bio-rad plate is version 4.
Upload to the app and it should pass analysis.
Changelog
create the json file
import into api and JS
create the JS util that returns the disallowed labware def versions based on the given api level
plug into PD's get latest version util
Risk assessment
low, merged into edge