Skip to content

DOC: Update documentation of Installing the development version of scikit-learn #31173

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

vitorpohlenz
Copy link
Contributor

@vitorpohlenz vitorpohlenz commented Apr 10, 2025

Reference Issues/PRs

Fixes #31149

What does this implement/fix? Explain your changes.

Changed a few lines in the conda environments (suggestion for not using the newest Python 3.13 version in Windows), and updated the Windows Build Tools path for compilers.

Any other comments?

It is my first PR in scikit-learn, so I tried to keep it really small

Copy link

github-actions bot commented Apr 10, 2025

✔️ Linting Passed

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

Generated for commit: 674aba2. Link to the linter CI: here

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @vitorpohlenz. Here are some comments and nitpicks

Comment on lines 74 to 85
.. note::

At the time of writing, the newest Python 3.13 version from the conda distribution uses freethreading,
which may cause problems when compiling on some OS like Windows.
In these cases, it is recommended to use Python 3.12 in the `conda environment`. With the following command:

.. prompt:: bash $

conda create -n sklearn-env -c conda-forge python=3.12 numpy scipy cython meson-python ninja

If the situation above is not the case, you can use the latest versions with the following command:

Copy link
Member

Choose a reason for hiding this comment

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

I would not include that in the doc. This is a temporary issue with conda-forge and the workaround is pretty straightforward. (by the way you can still install python 3.13 by requesting the python-gil package additionaly). This is the kind of stuff that stick in the doc for too long and we forget to clean it up and it clutters the doc and creates confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really understand what you are saying, sometimes "temporary" recommendations stick as permanents. But I just wanted to give some warning about this version that uses free-threeding (not sure if in the following versions this error will persist).
But it may be the case to wait for that to see, right?
I can remove it, no problem, but if you see any way to give this "temporary warning," just let me know and I will adjust in this doc.

Comment on lines 122 to 123
#. Build the project with pip,
whether you are in `conda environment` or in dedicated `virtualenv`:
Copy link
Member

Choose a reason for hiding this comment

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

I would not change that. This is the next step after creating and activating the env. I don't see how it could be ambiguous here. I'm probably biased but reading this makes me wonder "why is there a need for this precision ? Is there something I missed ?".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would not change that. This is the next step after creating and activating the env. I don't see how it could be ambiguous here. I'm probably biased but reading this makes me wonder "why is there a need for this precision ? Is there something I missed ?".

Probably, I also have some bias, which was the reason for me to update that line. As my first time building from source using conda env, I was forgetting this step in the conda env, and just executing this step when in the virtualenv.

But it was probably more related to my lack of attention in this part than a problem with the tutorial.

vitorpohlenz and others added 4 commits April 11, 2025 13:20
Ajusting note comment sugested by jeremiedbb when using Windows Build Tools

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>
… file and the specific python 3.13 version in conda env
@vitorpohlenz
Copy link
Contributor Author

Thanks for the PR @vitorpohlenz. Here are some comments and nitpicks

Thanks for the comments @jeremiedbb, since it was my first PR in sklearn, I found it nice to think about what you suggested, even the "nitpicks".

Also, I would like to ask for advice for this and future PRs:

Should I click on the button Re-request review ?

I do not want to be annoying like "I want you to review it again now," but this also could be used as a flag like "Everything alright, I have finished working in the comments, so you can review it again when you have time."

@jeremiedbb
Copy link
Member

Should I click on the button Re-request review ?

I can't answer for the other maintainers. I don't mind the request review button. You can also directly ping me, that's what we usually use. In any case, we might not answer right away because the review bandwidth is limited on the project.

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@jeremiedbb jeremiedbb enabled auto-merge (squash) April 12, 2025 16:51
@vitorpohlenz
Copy link
Contributor Author

Should I click on the button Re-request review ?

I can't answer for the other maintainers. I don't mind the request review button. You can also directly ping me, that's what we usually use. In any case, we might not answer right away because the review bandwidth is limited on the project.

Thanks for the tip @jeremiedbb !

I will do this in the next PRs!

@jeremiedbb jeremiedbb merged commit cc28738 into scikit-learn:main Apr 12, 2025
36 checks passed
@@ -186,7 +186,11 @@ commands in ``cmd`` or an Anaconda Prompt (if you use Anaconda):
.. prompt:: bash C:\>

SET DISTUTILS_USE_SDK=1
"C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" x64
"C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" x64
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if this is still needed now that we use meson ... the DISTUTILS_USE_SDK is not needed anymore with 99% probability, the vcvarsall.bat is probably not needed.

Personally, on Windows I compile inside a miniforge prompt and I never do this ...

It would be great if you could confirm @vitorpohlenz!

Copy link
Member

Choose a reason for hiding this comment

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

I also tried to install CPython from python.org and use a venv in a cmd console and same thing it works fine without setting DISTUTILS_USE_SDK and vcvarsall.bat. I opened #31202 to simplify the Windows instructions.

Copy link
Contributor Author

@vitorpohlenz vitorpohlenz Apr 14, 2025

Choose a reason for hiding this comment

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

@lesteve, it seems that you are right!

I have tried creating the environment again without activating or calling the VS Environment for the compiler, and the Meson Build seems to handle it quite well.

Here is the Full log of the build. You can see that in line 15 it activates the VS Env, when running the command:

pip install --editable . --verbose --no-build-isolation --config-settings editable-verbose=true

Without the :

SET DISTUTILS_USE_SDK=1

"C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvarsall.bat" x64

The same message appears whether using a conda-env or virtualenv.

Summarizing, it seems that if you installed the Microsoft Visual Studio Build Tools correctly, the Meson Build will handle these things, and these commands are not necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @vitorpohlenz for confirming this!

@lesteve lesteve added the OS:Windows Problem specific to Windows label Apr 16, 2025
lucyleeow pushed a commit to EmilyXinyi/scikit-learn that referenced this pull request Apr 23, 2025
…kit-learn#31173)

Co-authored-by: Jérémie du Boisberranger <jeremie@probabl.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation OS:Windows Problem specific to Windows
Projects
None yet
3 participants