Optional deps should be optional#941
Merged
rickwierenga merged 13 commits intoPyLabRobot:mainfrom Mar 18, 2026
Merged
Conversation
pylabrobot/visualizer/__init__.py
Outdated
Comment on lines
+1
to
+4
| try: | ||
| from pylabrobot.visualizer.visualizer import Visualizer | ||
| except ImportError: | ||
| pass |
Member
There was a problem hiding this comment.
I think if importing the visualizer leads to errors, we should catch them in visualizer.py rather than here
pylabrobot/storage/__init__.py
Outdated
Comment on lines
+3
to
+15
| from .cytomat import CytomatBackend | ||
| try: | ||
| from .cytomat import CytomatBackend | ||
| except ImportError: | ||
| pass | ||
| from .incubator import Incubator | ||
| from .inheco.scila import SCILABackend | ||
| from .liconic import ExperimentalLiconicBackend | ||
| try: | ||
| from .inheco.scila import SCILABackend | ||
| except ImportError: | ||
| pass | ||
| try: | ||
| from .liconic import ExperimentalLiconicBackend | ||
| except ImportError: | ||
| pass |
Member
There was a problem hiding this comment.
similar to visualizer: if these files cannot be imported, that should be fixed in those files rather than the init
Contributor
Author
There was a problem hiding this comment.
Ok, I'll give it a try. I guess there are three different levels of support for "unavailable" backends (i.e. with missing dependencies):
- Cannot even be imported (Current state of this PR).
- Can be imported but not instantiated.
- Can be imported and instantiated, but
.setupfails.
The main implication of not going with 1) above would be that we cannot generally import types from optional dependencies for type annotations, at least not without some sort of fallback.
Member
There was a problem hiding this comment.
Can be imported but not instantiated.
this is the best I think
for typing we can use TYPE_CHECKING
Contributor
Author
There was a problem hiding this comment.
I didn't want to test the full product combination of all python versions and all dependency configurations of interest, because the number of tests quickly explodes. Options that I considered:
- Writing all tests to be performed as the sum of two matrices of configurations. Unfortunately, not possible with GHA syntax (unlike GitLab CI).
- Having one matrix e.g. for the python version, and
include:all other configurations individually.
Rejected as not DRY, with all other configurations beside the dependencies needing to be repeated. - Making two completely separate tests, one with the matrix for python versions, the other with dependency configs. Rejected as not DRY, duplicated test code.
- Making two tests that include a common test code (either via
uses:as in this PR, orworkflow_dispatch). Implementation in this PR. - Programmatically generating all configurations to be tested in a separate step using a bit of bash or python, then feeding the configuration as inputs into the matrix. Viable alternative. My coding agent selected the previous option, but I was indifferent to the two.
Would you prefer the last option?
…tras Co-authored-by: Rick Wierenga <rick_wierenga@icloud.com>
…bugifx/optional-deps-should-be-optional
…mports Optional deps (serial, pylibftdi, hid, usb, websockets, pymodbus) are now wrapped in try/except at module level so imports always succeed. The runtime check raises in __init__() instead of setup(), giving an immediate clear error when the class is instantiated. Reverts the try/except band-aids in __init__.py re-export files since they are no longer needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap serial import in keyence_backend, heraeus_cytomat_backend, a4s_backend, xpeel_backend with try/except + __init__ check - Fix import sorting in 6 test files - Guard agrowdosepump_tests.py and pico backend_tests.py with unittest.SkipTest when optional deps missing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The HID, USB, and Serial check-on-init broke tests that construct IO objects without calling setup() (e.g. Hamilton STAR tests, Byonoy resource tests). These checks must stay in setup() — the backend __init__ checks are sufficient for user-facing error messages. Also add type: ignore for numpy import in pico backend_tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rickwierenga
force-pushed
the
bugifx/optional-deps-should-be-optional
branch
2 times, most recently
from
62d8dc6 to
59f128d
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
|
thanks! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
On current
main, pylabrobot is not importable when installed with just the mandatory dependencies. This PRall__init__.pyfiles to fail