Skip to content

CI Move some pip_dependencies to conda_dependencies #31623

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

StefanieSenger
Copy link
Contributor

This PR updates update_environments_and_lock_files.py to use conda for packages that were installed via pip but are now available on conda.

Copy link

github-actions bot commented Jun 22, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 6f02648. Link to the linter CI: here

@StefanieSenger
Copy link
Contributor Author

This is a draft to be able to experiment.

I checked all packages in pip_dependencies if the required version is available on the conda channel that is used in this CI setting.

I was unsure and have not moved meson, because it was not explicitly mentioned in this comment and I assume there is a reason for it?

Other dependencies have not defined a version in our pyproject.toml and I have moved them to conda_dependencies regardless if they were available: lightgbm, array-api-strict, pytest-xdist, ninja, jupyterlite-sphinx, and jupyterlite-pyodide-kernel. That means they will probably come in different versions as with pypi and we'll see where this leads, when the CIs have run.

@StefanieSenger
Copy link
Contributor Author

StefanieSenger commented Jun 22, 2025

Okay, seems that the version change in array-api-strict, which has no specified version our pyproject.toml, but was 2.4 (edit: no it was 2.3.1) with pip, and would be 2.2 with conda defaults as required by thepylatest_pip_openblas_pandas CI caused some tests to fail. So I think I should move it back to the pip_dependencies.

@StefanieSenger
Copy link
Contributor Author

StefanieSenger commented Jun 22, 2025

I have moved array-api-strict back to the pip_dependencies, and now it shows up in the diff of the lock file for the first time (before it didn't which I thought was strange) as:

# pip array-api-strict @ https://files.pythonhosted.org/packages/fe/c7/a97e26083985b49a7a54006364348cf1c26e5523850b8522a39b02b19715/array_api_strict-2.3.1-py3-none-any.whl#sha256=0ca6988be1c82d2f05b6cd44bc7e14cb390555d1455deb50f431d6d0cf468ded
# pip array-api-strict @ https://files.pythonhosted.org/packages/e5/33/cede42b7b866db4b77432889314fc652ecc5cb6988f831ef08881a767089/array_api_strict-2.4-py3-none-any.whl#sha256=1cb20acd008f171ad8cce49589cc59897d8a242d1acf8ce6a61c3d57b61ecd14

That shows that before version 2.3.1 was used and now it is the newest version on pypi 2.4, that was released 6 days ago.
I don't know why it is not version-locked since it is a fast developing library, but let's hope now the CI passes.

Edit: Nice they all passed. 🎉

Now it's only the Windows CI failing, and this is unrelated and coming from main.

@StefanieSenger StefanieSenger marked this pull request as ready for review June 22, 2025 08:52
@betatim
Copy link
Member

betatim commented Jun 25, 2025

This PR updates update_environments_and_lock_files.py to use conda for packages that were installed via pip but are now available on conda.

Do you know why we used pip for these and not conda? Was it because the version we wanted were not available on conda-forge? I don't know the answer but I assume that (like so many things in scikit-learn) it was done for a specific reason. If we know the reason then it is less likely to break something by accident

@StefanieSenger
Copy link
Contributor Author

StefanieSenger commented Jun 25, 2025

@betatim: I didn't know this before, but @adrinjalali told me that's a maintanance task we do from time to time. It is necessary because the versions we need were not available (before) on the respective conda channel that is used for this job (some use conda-forge and other use conda defaults channel) - but now they might be and we prefer this.

I also took notes while I checked everything. Let me share these, maybe it helps with reviewing:

We have specific minimal version requirements in our pyproject.toml:

We do not have a version specified in pyproject.toml and have thus used the newest version available on pypi:

Whether I have checked the version from the main ("defaults") channel or the conda-forge channel depends on how the channel in the job was specified in build_tools/update_environments_and_lock_files.py. Did I do all that correctly 🤷 ?

@betatim
Copy link
Member

betatim commented Jun 25, 2025

Cool! Thanks for doing the "chopping wood and carrying water" on moving things from PyPI to conda.

@betatim
Copy link
Member

betatim commented Jun 25, 2025

As far as I can tell all the moves make sense. I think we should wait for the CI to be working again and run it again. Maybe even using the [cd build] marker in a commit message to run the builds?

"conda_dependencies": ["python", "ccache"],
"pip_dependencies": (
remove_from(common_dependencies, ["python", "blas", "pip"])
"conda_dependencies": (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this build's goal (note the pip in pylatest_pip_openblas_pandas) is to install everything with pip, and you don't want to move things to conda dependencies here.

@@ -234,7 +231,15 @@ def remove_from(alist, to_remove):
"folder": "build_tools/azure",
"platform": "linux-64",
"channels": ["defaults"],
"conda_dependencies": ["python", "ccache"],
"conda_dependencies": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here, the pip inside the build name means we want to install all Python packages with pip.

# TODO: put cython, threadpoolctl and meson-python back to conda
# dependencies when required version is available on the main channel
"pip_dependencies": ["cython", "threadpoolctl", "meson-python", "meson"],
"pip_dependencies": ["meson"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove meson here because meson-python depends on meson and meson-python is installed via the conda section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants