conda-forge / qutip-feedstock

A conda-smithy repository for qutip.
BSD 3-Clause "New" or "Revised" License
0 stars 14 forks source link

Cython dependency #53

Closed wshanks closed 3 years ago

wshanks commented 4 years ago

This conda recipe does not set Cython as a run dependency, but install_requires in qutip's setup.py does include Cython.

I know there is some history of discussion regarding this in #8 (and also that handling Cython in qutip has caused some problems like https://github.com/qutip/qutip/issues/1246 as well). It seems like the initial feedstock PR included Cython as a run dependency but the conda-forge reviewers discouraged that because it seemed unlikely Cython was really needed at run time. At the time, qutip had autocompile code that imported Cython and that was patched out in the recipe here. Now it seems that all Cython imports are wrapped in try's so it is okay to run qutip without Cython (but I have not studied how qutip uses Cython at run time in detail, so I am not positive; that is why I am opening an issue here rather than proposing removing Cython from qutip's install_requires).

My suggestion is that this feedstock should be consistent with the upstream qutip. So either both should specify Cython as a dependency or neither. Until a time when upstream qutip removes Cython as an install_requires dependency, I propose that this feedstock should specify it as a run dependency.

The mismatch in requirements does not produce too bad of outcomes, but there is some consequence. Personally, I try to keep my conda environments as purely composed of conda packages as possible but sometimes it is necessary to install packages with pip. I have found that recent versions of pip will install missing dependencies when you run other installs. So when I have the qutip conda package installed, I end up getting Cython installed by pip the next time I use pip because pip sees it as a missing dependency of qutip.

nathanshammah commented 4 years ago

Thanks @willsALMANJ, would you be able to propose a PR?

jakelishman commented 3 years ago

Fixed by the release of 4.6.0 in #63.