-
Notifications
You must be signed in to change notification settings - Fork 18
Switch Docker build from conda to UV, update dependencies #196
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
Conversation
Replaces conda-based Docker build scripts with UV (Rust-based package manager) for faster and more reliable builds. Updates Docker scripts, entrypoints, and documentation to use Python 3.11 and UV virtual environments. Updates minimum Python version to 3.11 in setup.py and README, and bumps several dependencies (arkitekt-next, mikro-next, koil, kabinet, fakts-next, platformdirs, numpy) in uv.lock. Removes benchmark_hologram_processing.py and fixes minor typos and instructions in documentation. Improves LiveViewController frame read logic and adds debug output to configfiletools.
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.
Pull request overview
This PR migrates the Docker build system from conda to UV (a Rust-based Python package manager) for improved build performance and reliability. The minimum Python version requirement is updated to 3.11, and several dependencies are bumped to newer versions.
Key Changes:
- Replace conda/Miniforge with UV for Python package management in Docker builds
- Update minimum Python version from 3.10 to 3.11
- Remove benchmark_hologram_processing.py script
- Fix minor documentation typos and improve LiveViewController frame handling
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Update minimum Python version to 3.11 and bump numpy to 2.1.2 |
| pyproject.toml | Clean up trailing whitespace in dependency list |
| imswitch/imcontrol/model/configfiletools.py | Add debug output by printing file contents before parsing |
| imswitch/imcontrol/controller/controllers/LiveViewController.py | Add frame read attempt counter and only stop after 3 failures |
| docs/uv-guide.md | Update UV lock file instructions and add conda migration guide |
| dockerfile | Switch from conda to UV build script and update PATH |
| docker/venv-shell.sh | Update activation to use UV virtual environment instead of conda |
| docker/entrypoint.sh | Update activation to use UV virtual environment |
| docker/build-imswitch.sh | Replace conda commands with UV pip commands |
| docker/build-imswitch-deps.sh | Replace conda package installation with UV pip |
| docker/build-drivers.sh | Update driver installation to use UV venv activation |
| docker/build-conda.sh | Replace Miniforge installation with UV setup (renamed from build-conda.sh to build-uv.sh) |
| docker/README.md | Add UV package management feature description |
| docker/CONFIG_GUIDE.md | Update build optimization documentation to reference UV |
| docker/.dockerignore | Add UV and Python cache directories to ignore list |
| benchmark_hologram_processing.py | Remove entire benchmark script file |
| README.md | Fix typo, update Python requirement to 3.11, add UV launch instructions |
| .github/workflows/imswitch-bundle.yml | Update to Python 3.11 and add UV setup action |
| .dockerignore | Add comprehensive Docker ignore patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| mFile = setupFile.read() | ||
| print(mFile) | ||
| mSetupDescription = setupInfoType.from_json(mFile, infer_missing=True) |
Copilot
AI
Nov 28, 2025
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.
Debug print statement should be removed or replaced with proper logging. Use self._logger.debug(mFile) instead of print(mFile) for consistency with the logging pattern used elsewhere in the codebase (e.g., line 62).
| self._logger.warning("Frame capture failed, stopping worker") | ||
| self._running = False | ||
| break | ||
| frameReadAttempts += 1 |
Copilot
AI
Nov 28, 2025
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.
frameReadAttempts counter is never reset after successful frame capture. This means once it exceeds 3, all subsequent frame failures will immediately stop the worker. The counter should be reset to 0 after a successful frame read (e.g., after line 118 or when frame is not None).
| rm -rf /opt/conda/pkgs/* | ||
| rm -rf /root/.cache/uv | ||
| rm -rf /root/.cache/pip | ||
| rm -rf /tmp/*onda/pkgs/* |
Copilot
AI
Nov 28, 2025
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.
Corrected typo in path from '/tmp/onda/pkgs/' to '/tmp/'. The path appears to be a remnant from conda cleanup and should just be '/tmp/' as shown in line 105.
| rm -rf /tmp/*onda/pkgs/* |
| rm -rf /tmp/*onda/pkgs/* | ||
| pip3 cache purge || true | ||
| rm -rf /root/.cache/pip |
Copilot
AI
Nov 28, 2025
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.
Duplicated cleanup commands. Line 101 and 104 both execute 'rm -rf /root/.cache/pip', and line 102 and 105 both clean /tmp (though line 102 has a typo). Remove the duplicate cleanup commands on lines 102-104.
| rm -rf /tmp/*onda/pkgs/* | |
| pip3 cache purge || true | |
| rm -rf /root/.cache/pip | |
| pip3 cache purge || true |
ethanjli
left a comment
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.
The changes seem reasonable at first glance. In a future PR, we may want to convert some manual uv pip install commands into dependency entries pyproject.toml (perhaps using the dependency groups feature if necessary).
Next week I can check whether any additional cache data will need to be removed from the container image.
|
Sounds good. Thanks! |
I don't see any large files/directories of cache data in the container image. It appears that this PR has decreased the container image size by a significant amount, from ~4 GB to 3 GB. According to dive, the build scripts make the following contributions to image size in commit 212570d (currently the HEAD of the master branch):
According to dive, the build scripts make the following contributions to image size in this PR:
For some reason, although |
|
Impressive! We could potentially save some more memory id we use uv over conda? |
|
I would be extremely surprised if we saved any RAM, but we're clearly seeing savings of disk space (and thus, presumably, SD card image size). I guess either 1) the way we install things with UV is more efficient than with Conda due to the design of UV, or 2) we were making a mistake in how we were installing things with Conda. Explanation 2 sounds more plausible to me, since there's a lot I didn't understand about how we were installing things with Conda ;P |
|
I'm a bit puzzled about this PR now. Since we can use UV already in the master branch for installing ImSwitch locally, we could close it for now (?). This PR is mostly for replacing conda with uv in the docker build process, right? |
The most important change this PR makes is to replace Conda with UV in the Docker build process. We must complete this change so that our development environment (based on UV) is not wildly different from our production environment (currently based on Conda). The more the divergence, the more the future headaches for us once something goes wrong. We should merge the replacement of Conda with UV in the Docker build process (or you can close this PR and create a new PR which performs that replacement); I'm fine if you discard any other changes you've made in this PR (e.g. something about |
|
Sure. Sorry. I guess I needed a break :D |
|
Sure, just tag me in a new comment here once you're ready for me to test in a new PR on rpi-imswitch-os |
|
@ethanjli seems like the test went through. So we could boot up a container and see if imswitch using uv is properly working ? |
This PR integrates openUC2/ImSwitch#208 and openUC2/ImSwitch#196 .
|
The ImSwitch backend server starts and serves the React frontend, so I assume that uv is working correctly |
Replaces conda-based Docker build scripts with UV (Rust-based package manager) for faster and more reliable builds. Updates Docker scripts, entrypoints, and documentation to use Python 3.11 and UV virtual environments. Updates minimum Python version to 3.11 in setup.py and README, and bumps several dependencies (arkitekt-next, mikro-next, koil, kabinet, fakts-next, platformdirs, numpy) in uv.lock. Removes benchmark_hologram_processing.py and fixes minor typos and instructions in documentation. Improves LiveViewController frame read logic and adds debug output to configfiletools.
@jhnnsrs interested to review this?
@ethanjli I migrated everything to uv and removed conda from docker. Was on my list for quiet some time. I have not yet checked if picamera2 still works. Probably not... What do you think?