-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
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
Installation guidelines now display default pip install commands and only expands in the notes #27960
Conversation
…nly hints at additional optimized parameters in a note.
@@ -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 . |
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.
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.
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.
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 🤔
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.
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)
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.
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.
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 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
.
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.
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 tosys.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.
I confirm however that in my environment (windows 11 VM, miniforge prompt, mamba, separate environments, etc) not passing |
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.
both those flags are required if we don't want contributors to wait for a complete build everytime they run the command. Otherwise their workflow will be very slow.
If we were comfortable recommending people to use main in
then we'd be in an easier place.
During springs, almost always we end up having to install WSL for contributors on windows. So building under windows should be considered advanced and we can have windows specific commands in the advanced installation section. But by default we shouldn't change the flags which makes everybody else's workflow significantly slower.
I'm -1 on this.
FYI I opened #27806 which should fix the weird pytest issue on Windows. |
Let's close this. I see the need of a practical guide for sprints with beginners. I hope it does not slow the adoption of good environment management skills. Anyway I don't see a way out to improve that right now. |
What does this implement/fix? Explain your changes.
Change the developers documentation to primarily showcase usage of pip with default flags rather than always with
--no-build-isolation
and--no-use-pep517
. Only suggests those flags in side notes. Why:--no-use-pep517
is itself described as a temporary workaround for a bug ~ a bit worryingSo I'm proposing to re-write it a way that hints that usage of those flags are for more experienced developers that need to optimize their workflow, and suggests that the default usage should be preferred for simpler cases or bug reports (maybe this could be explicitly stated too ?)