8000 Installation guidelines now display default pip install commands and only expands in the notes by fcharras · Pull Request #27960 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

Installation guidelines now display default pip install commands and only expands in the notes #27960

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 25 additions & 20 deletions doc/developers/advanced_installation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ feature, code or documentation improvement).

.. prompt:: bash $

pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .
Copy link
Member

Choose a reason for hiding this comment

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

But the --no-build-isolation seems important, isn't it?

Otherwise, you are going to build with other version of library that are isolated and not use the one from your current environment.

If we go this way, we never need to request Cython for instance and only NumPy and SciPy to be installed for the runtime.

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 don't know either how build isolation works exactly. But if that's true maybe the doc shouldn't hint that --no-build-isolation is for saving time only then 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Saving time for me it just a side effect of what is happening. The parameter allows you to:

  • control the version of library used to compiled since you are not creating an isolated environment
  • you will not recompile everything from scratch since you leverage the already compiled files since you are not isolated

Perfectly, we should not have to explain this in our documentation but have the proper explanation on the pip documentation.

I assume that the part about the PEP517 is something that we never really documented: #26334 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Running twice pip install -e . --no-build-isolation, I can see that pip will create a tmp folder where to have the build and the compilation will happen at each call. Adding the --no-use-pep-517 force to build into the build folder of the repository directly and it will not recompile if there is any change.

Copy link
Member

Choose a reason for hiding this comment

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

The way I understand --no-build-isolation is as "use the libraries available in the environment in which pip is being run". It seems odd to me that you need to specify this, as an old person I think "what else would I want pip to do??", but times move on and I assume there is a good reason why the default isn't --no-build-isolation.

tl;dr: I think we should keep it, and we (as developers) understand exactly why it is there in the docs.

Until I read Guillaume's last comment here I was under the impression that no one fully understood what --no-use-pep517 does. But from his comment it seems like he does. The naming is a bit weird, especially if you don't know your PEPs off by heart, but it seems it does something useful and we can explain it.

Should we explain it? I don't know, maybe link to the pip docs?

As long as we don't know of any negative side-effects for the unsuspecting developer, I think we should keep the command as is (and feel sad that the names are a bit abstract). The windows build issue might be a reason to remove --no-use-pep517.

Copy link
Contributor Author
@fcharras fcharras Dec 14, 2023

Choose a reason for hiding this comment

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

build isolation
For building packages using this interface, pip uses an isolated environment. That is, pip will install build-time Python dependencies in a temporary directory which will be added to sys.path for the build commands. This ensures that build requirements are handled independently of the user’s runtime environment.

Isn't there to understand that build dependencies are temporarily fetched into a separate, temporary file tree if requirements are not already met by the user runtime environment ? I don't find any documentation of it actually having to do with compiled extensions and such.

"what else would I want pip to do??"

I'd say that in general using different environments for different purposes is good, avoid a lot of troubles and unexpected interactions caused by unsuspected incompatible dependency trees and such, having the build system alter the dependencies of the environment that will be used at runtime is one of those instances of mixing things ~ especially for packages that can have build time dependencies that are not also runtime dependencies.


#. Check that the installed scikit-learn has a version number ending with
`.dev0`:
Expand All @@ -110,14 +110,19 @@ feature, code or documentation improvement).

.. note::

You will have to run the ``pip install -v --no-use-pep517 --no-build-isolation -e .``
command every time the source code of a Cython file is updated
(ending in `.pyx` or `.pxd`). This can happen when you edit them or when you
use certain git commands such as `git pull`. Use the ``--no-build-isolation`` flag
to avoid compiling the whole project each time, only the files you have
modified. Include the ``--no-use-pep517`` flag because the ``--no-build-isolation``
option might not work otherwise (this is due to a bug which will be fixed in the
future).
You will have to run the ``pip install -v -e .`` command every time the source code
of a Cython file is updated (ending in `.pyx` or `.pxd`). This can happen when you
edit them or when you use certain git commands such as `git pull`. In order to save
time, you can compile the files you have modified only rather than the whole project
using` the ``--no-build-isolation`` flag. Then also include the
``--no-use-pep517`` flag because the ``-no-build-isolation`` option might not work
otherwise (this is due to a bug which will be fixed in the future). Thus the full
recommended command in this case will be:

.. prompt:: bash $

pip install -v -e . --no-build-isolation --no-use-pep517


Dependencies
------------
Expand Down Expand Up @@ -188,9 +193,9 @@ Editable mode

If you run the development version, it is cumbersome to reinstall the package
each time you update the sources. Therefore it is recommended that you install
in with the ``pip install -v --no-use-pep517 --no-build-isolation -e .`` command,
which allows you to edit the code in-place. This builds the extension in place and
creates a link to the development directory (see `the pip docs
in with the ``pip install -v -e .`` command, which allows you to edit the code in-place.
This builds the extension in place and creates a link to the development directory (see
`the pip docs
<https://pip.pypa.io/en/stable/topics/local-project-installs/#editable-installs>`_).

As the doc above explains, this is fundamentally similar to using the command
Expand Down Expand Up @@ -253,7 +258,7 @@ Finally, build scikit-learn from this command prompt:

.. prompt:: bash $

pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

.. _compiler_macos:

Expand Down Expand Up @@ -301,7 +306,7 @@ activating the newly created conda environment.

conda activate sklearn-dev
make clean
pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

.. note::

Expand Down Expand Up @@ -375,7 +380,7 @@ Finally, build scikit-learn in verbose mode (to check for the presence of the
.. prompt:: bash $

make clean
pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

.. _compiler_linux:

Expand All @@ -401,7 +406,7 @@ then proceed as usual:
.. prompt:: bash $

pip3 install cython
pip3 install --verbose --editable .
pip3 install -v -e .

Cython and the pre-compiled wheels for the runtime dependencies (numpy, scipy
and joblib) should automatically be installed in
Expand Down Expand Up @@ -441,7 +446,7 @@ activating the newly created conda environment.
.. prompt:: bash $

conda activate sklearn-dev
pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

.. _compiler_freebsd:

Expand Down Expand Up @@ -470,7 +475,7 @@ Finally, build the package using the standard command:

.. prompt:: bash $

pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

For the upcoming FreeBSD 12.1 and 11.3 versions, OpenMP will be included in
the base system and these steps will not be necessary.
Expand All @@ -491,7 +496,7 @@ The following command will build scikit-learn using your default C/C++ compiler.

.. prompt:: bash $

pip install --verbose --editable .
pip install -v -e .

If you want to build scikit-learn with another compiler handled by ``setuptools``,
use the following command:
Expand Down Expand Up @@ -531,7 +536,7 @@ and environment variable as follows before calling the ``pip install`` or
``python setup.py build_ext`` commands::

export SKLEARN_BUILD_PARALLEL=3
pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

On a machine with 2 CPU cores, it can be beneficial to use a parallelism level
of 3 to overlap IO bound tasks (reading and writing files on disk) with CPU
Expand Down
13 changes: 10 additions & 3 deletions doc/developers/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,17 @@ The next steps now describe the process of modifying code and submitting a PR:

.. prompt:: bash $

pip install -v --no-use-pep517 --no-build-isolation -e .
pip install -v -e .

Use the ``--no-build-isolation`` flag to avoid compiling the whole project
each time, only the files you have modified.
In order to save time, you can compile the files you have modified only rather than
the whole project, using the ``--no-build-isolation`` flag.Then also include the
``--no-use-pep517`` flag because the ``-no-build-isolation`` option might not work
otherwise (this is due to a bug which will be fixed in the future). The full
recommended command in this case will be:

.. prompt:: bash $

pip install -v -e . --no-build-isolation --no-use-pep517

It is often helpful to keep your local feature branch synchronized with the
latest changes of the main scikit-learn repository:
Expand Down
0