Skip to content
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

On anaconda_conda #560

Closed
tkf opened this issue Sep 8, 2018 · 1 comment · Fixed by #580
Closed

On anaconda_conda #560

tkf opened this issue Sep 8, 2018 · 1 comment · Fixed by #580

Comments

@tkf
Copy link
Member

tkf commented Sep 8, 2018

I propose to stop unconditionally installing conda packages in pyimport_conda when Conda.jl is not used:

PyCall.jl/src/PyCall.jl

Lines 657 to 663 in 49029e0

else
aconda = anaconda_conda()
if !isempty(aconda)
try
Compat.@info "Installing $modulename via Anaconda's $aconda..."
isempty(channel) || run(`$aconda config --add channels $channel --force`)
run(`$aconda install -y $condapkg`)

If a user explicitly sets $PYTHON then it means s/he do not want auto-magical handling of Python packages by Conda.jl. Installing package automatically at runtime is not what programmers would expect, I hope (unless, say, there is some integrity check like GPG verification). I can think of several options to fix it:

  1. Let CONDA_JL_HOME handle this case and remove this if !isempty(aconda) branch.
  2. Print an error message with instruction to install appropriate packages.
  3. If isinteractive() then prompt user before executing conda install.

If you are going to take an option like 3, I think there is one more thing to be fixed:

PyCall.jl/src/PyCall.jl

Lines 662 to 663 in 49029e0

isempty(channel) || run(`$aconda config --add channels $channel --force`)
run(`$aconda install -y $condapkg`)

I don't think any package should modify user's configuration silently. Why not use conda install --channel $channel ... like this?:

options = ``
if isempty(channel)
    options = `--channel $channel`
end
run(`$aconda install -y $options $condapkg`)
@stevengj
Copy link
Member

I'm okay with removing the if !isempty(aconda) branch and just replacing it with an exception that suggests what conda command to run.

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

Successfully merging a pull request may close this issue.

2 participants