update pip command #618
update pip command #618
Conversation
| @@ -105,7 +105,7 @@ Pure Python packages almost never need them. | |||
|
|
|||
| If the build can be executed with one line, you may put this line in the | |||
| ``script`` entry of the ``build`` section of the ``meta.yaml`` file with: | |||
| ``script: python -m pip install --no-deps --ignore-installed . --verbose``. | |||
| ``script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed --no-cache-dir -vvv"``. | |||
There was a problem hiding this comment.
Why do we have {{ PYTHON }} here?
There was a problem hiding this comment.
This matches the script string on the new conda-skeleton and my guess is that {{ PYTHON }} is safer than python b/c ensure the env variable for is used. Is that correct @msarahan?
There was a problem hiding this comment.
It ensures that the host env Python is always used. It is equivalent to using the env var, but works on both win and Unix.
There was a problem hiding this comment.
Is there any documentation on {{ PYTHON }} and friends?
If I read
https://github.com/conda/conda-build/blob/3.12.0/conda_build/jinja_context.py#L496
https://github.com/conda/conda-build/blob/3.12.0/conda_build/environ.py#L253
https://github.com/conda/conda-build/blob/3.12.0/conda_build/environ.py#L323-L332
correctly, {{ PYTHON }} has the backslashes escaped for windows paths and thus it is correct (and even necessary) to use it in a double quote-style YAML scalar, correct?
NB: Looking at this, the increasing list of options and more importantly things like pypa/pip#5605, it would be very advantageous (for robustness' sake) to have something like script: fancy-conda-python-package-installer that takes care of all those things... ref: #611
There was a problem hiding this comment.
Completely agree, @mbargull. How about script: {{ compiler('python') }}?
There was a problem hiding this comment.
I strongly disagree that creating another layer is necessary. The more transparent and 1-1 correspondence with what regular pip user can do the better to avoid passing the image that conda is this obscure fork-ish tool.
In that case, why not keep python instead of {{ PYTHON }}? Already conda activates the correct environment putting the correct python on the path. Am not aware of any issues that just using python in the recipe has caused.
There was a problem hiding this comment.
From @msarahan message above:
It ensures that the host env Python is always used. It is equivalent to using the env var, but works on both win and Unix.
There was a problem hiding this comment.
@mbargull it works just fine the way it is, see: https://ci.appveyor.com/project/conda-forge/spglm-feedstock/branch/master/job/6h840ke8uy2kqfbs#L470
Thanks for confirming ❤️
How about script:
{{ compiler('python') }}
I don't think this abstraction/indirection is needed necessarily; just a "fancy-conda-python-package-installer" package/script/binary for starters.
I strongly disagree that creating another layer is necessary.
No hard feelings 😉.
Many of conda-forge users like to read a recipe and understand what is going on without having to be experts on conda/conda-forge internals.
IMHO, conda-build does some domain-specific things that I argue
- it doesn't necessarily have to do and makes it more complicated than the general purpose build tool it could be, and
- might make it less transparent to the non "conda/conda-forge internal"-expert.
I usually favor more abstract/general approaches (e.g., move Python-specific things out of conda-build and into a dedicated package) -- but of course with limits, i.e., I agree that unnecessary indirections that only complicate things should be avoided. In the end, most beauty lies in simplicity; and the proposed slight indirection seems the more simple and maintainable solution to me.
Note that all these pip options have corresponding env variables that can be set. We can teach conda-build to set these
That's more of the (potentially) intransparent domain-specific "black magic" I'd like to avoid putting into conda-build itself and rather have it (from my POV) a little more explicitly exposed in a separate tool.
In that case, why not keep
pythoninstead of{{ PYTHON }}? Alreadycondaactivates the correct environment putting the correctpythonon the path. Am not aware of any issues that just usingpythonin the recipe has caused.
Because of some reasons, the build environment is activated on top of the host one. Meaning, if you add some-python-tool to requirements/build, python will be in build and thus be the one at the top of PATH.
BUT: My "NB" was mostly only meant to reference gh-611 -- we shouldn't have this discussion here, but rather over at the linked or a new issue 😉.
There was a problem hiding this comment.
I don't like adding a new Jinja2 function where it isn't necessary. I agree that having a substitution is helpful here, but see no reason why it should be anything more than just another variable in your CBC.yaml file. Since we are already setting env vars rather globally to control pip (disable build isolation, conda/conda-build#3053), I also wonder how much value any further variable even adds.
There was a problem hiding this comment.
Note that all these pip options have corresponding env variables that can be set. We can teach conda-build to set these
That's what PR ( conda/conda-build#3053 ) is meant to do.
|
It looks like the discussion drifter from the original proposal to get our docs consistent with conda-skeleton and staged-recipes to a more general issue of improving conda-build. I'm merging this to ensure consistent among the install |
update pip command
Ping @marcelotrevisani
[skip ci]