remove Liquid tracking from volume tracker, delete contamination tracker #744
Conversation
This file contains hidden or 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
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request removes liquid tracking from the volume tracker and deletes the cross-contamination tracker, simplifying the API to track only volumes instead of individual liquids with their types.
Key Changes:
- Refactored
VolumeTrackerto track simple volume values instead of lists of liquid tuples - Removed all cross-contamination tracking functionality and associated helper functions
- Updated API surface with new methods (
set_volume,set_well_volumes) while deprecating old methods (set_liquids,set_well_liquids)
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
pylabrobot/resources/volume_tracker.py |
Core refactoring: replaced liquid tracking with simple volume tracking; removed cross-contamination functionality; added deprecation warnings for old API |
pylabrobot/resources/well.py |
Added set_volume method; deprecated set_liquids with warning |
pylabrobot/resources/tube.py |
Added set_volume method; deprecated set_liquids with warning |
pylabrobot/resources/plate.py |
Added set_well_volumes method; deprecated set_well_liquids method |
pylabrobot/resources/errors.py |
Added deprecation warning to CrossContaminationError.__init__ |
pylabrobot/resources/__init__.py |
Removed exports for deleted cross-contamination tracking functions |
pylabrobot/liquid_handling/standard.py |
Removed liquids field from aspiration/dispense dataclasses |
pylabrobot/liquid_handling/liquid_handler.py |
Removed liquid tracking and cross-contamination logic from aspirate/dispense operations |
pylabrobot/liquid_handling/backends/ |
Updated backends to remove liquid class detection from operations; simplified to default to Liquid.WATER |
pylabrobot/server/liquid_handling_server.py |
Removed liquid deserialization from API endpoints |
pylabrobot/visualizer/lib.js |
Simplified Container class to track single volume value instead of liquids array |
pylabrobot/visualizer/visualizer_tests.py |
Updated test to use new set_well_volumes API |
| Test files | Updated all tests to use volume-based API (set_volume, tracker.volume) instead of liquid-based API |
| Documentation notebooks | Updated examples to demonstrate new volume-based API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Raises: | ||
| ValueError: If the number of liquids does not match the number of wells in the plate. | ||
| volume: The volume to fill each well with, in uL. |
There was a problem hiding this comment.
The docstring mentions volume (singular) but the parameter is volumes (plural, a list). The description should be updated to clarify that this method accepts a list of volumes, one for each well.
Comment on lines
+189
to
+197
| def set_well_liquids( | ||
| self, | ||
| liquids: Union[ | ||
| List[List[Tuple[Optional["Liquid"], Union[int, float]]]], | ||
| List[Tuple[Optional["Liquid"], Union[int, float]]], | ||
| Tuple[Optional["Liquid"], Union[int, float]], | ||
| ], | ||
| ): | ||
| """Deprecated: Use `set_well_volumes` instead.""" |
There was a problem hiding this comment.
The deprecated set_well_liquids method is missing a deprecation warning. Since set_liquids in VolumeTracker emits a DeprecationWarning (lines 79-84 in volume_tracker.py), this method should also emit a deprecation warning to inform users that they should migrate to set_well_volumes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
https://discuss.pylabrobot.org/t/are-people-using-the-cross-contamination-tracker/370
the syntax is useless and it does not work well, so just delete it. it's better to not have it than have it whack