-
Notifications
You must be signed in to change notification settings - Fork 8
Added UV support in pip-containerize #37
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
base: master
Are you sure you want to change the base?
Conversation
for more robust site-packages path detection. Some linting fixes and minor mamba fix also included.
|
Hi, A basic test to validate that the uv support works would be a nice addition. The current tests are quite a mess, so any kind of script which one can run to validate that the basic feature works as intended would suffice (placed in Thinking out loud, as I don't know how often it is done, but is conda + uv often enough used to also introduce that as an option to |
- conda-containerize can now also do uv for pip packages - consolidated templates for venv/uvenv and conda, removing the specific ones for updates - included some tests for the new functionality
|
I have added an additional commit hopefully addressing your comments. Mamba nowadays supports uv out of the box if uv is part of the dependencies in the environment, so it has not been difficult to add that possibility here too. I added some tests for that functionality as well. I also took the opportunity to tidy up the templates, there was a lot of duplicated code for the new/ update operations. Now both use the same venv/conda templates. For the slim option, note that I removed the hardcoded python 3.12 for a "slim" default, replacing it by the latest available slim container. This is more in line with the default behaviour of tools like conda or uv. |
|
Would it be possible to separate out your coding style -related changes from the rest @xavierabellan? Currently this is a big pull request, but mostly style / whitespace changes, so it is difficult to review. |
|
Many editors/IDEs these days automatically remove trailing white spaces and format python properly on save. Unfortunately this code base does not follow any standard formatting and suffers of those problems, so it was a bit of a manual effort to go against it and leave it as close as it was with just the bits that needed changing for the feature, but here you go. |
|
Great, thanks! Will be tested. You're right that the project could also have consistent formatting, but it's just hard to review a commit with a lot of unrelated changes. |
Traubert
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.
Some initial observations:
- Running on our Puhti machine, with the "puhti" config enabled, two tests fail: "Package added in update available" and "Package added in uv update available". Did they work for you in the "local" config, or where?
- In eg. pip-containerize.py you have eg.
python_mode = parser_new.add_mutually_exclusive_group()
python_mode.add_argument("--uv", action="store_true", help="Use uv")
These don't show up at least with -h. When are they intended to show up? Currently the tools don't really say anything about --uv the way I try to use them.
- Prior to this PR, we trust one script to be downloaded to be downloaded from the Internet and executed, namely https://github.com/conda-forge/miniforge/releases/download/$CW_CONDA_VERSION/Miniforge3-$CW_CONDA_VERSION-$CW_CONDA_ARCH.sh. This adds another one, https://astral.sh/uv/install.sh. Not necessarily a bad thing, just wondering what is our security thinking about these.
UV is becoming more and more popular as a Python package manager due to its speed and compatibility with pip.
This PR adds the option to use uv instead of the standard python3 venv to create the environments. It is implemented as an alternative to the slim mode, installing uv and the requested/default python version managed by uv itself before creating the environment.
Also included in this PR, some other minor improvements that were picked up on the way: