Skip to content

Install all install packages from setup.py from conda-forge instead of pip#1542

Closed
valeriupredoi wants to merge 15 commits intomainfrom
all_conda_forge
Closed

Install all install packages from setup.py from conda-forge instead of pip#1542
valeriupredoi wants to merge 15 commits intomainfrom
all_conda_forge

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Mar 16, 2022

Description

Closes #1541

This the move of all packages that were previously in setup.py: install and were installed from PyPi to environment.yml and hence installed from and only from conda-forge. @zklaus suggested we use the - nodefaults channel option so that we get all those deps only from the specified channel (conda-forge in this case) and it's a good suggestion since I had already encountered an issue with a secondary dep being poorly installed from main. Ready for your consideration, folks 👍


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@valeriupredoi valeriupredoi added enhancement New feature or request installation Installation problem labels Mar 16, 2022
@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #1542 (721ff4b) into main (4dbac04) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1542   +/-   ##
=======================================
  Coverage   90.76%   90.76%           
=======================================
  Files         197      197           
  Lines       10568    10568           
=======================================
  Hits         9592     9592           
  Misses        976      976           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dbac04...721ff4b. Read the comment docs.

Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
@valeriupredoi
Copy link
Contributor Author

OK the OSX import error is actually not related to MyProxyClient import per se, but rather an issue with SSL:

from myproxy.client import MyProxyClient

Traceback (most recent call last):
  File "/home/valeriu/ESMValCore/testimp.py", line 1, in <module>
    from myproxy.client import MyProxyClient
  File "/home/valeriu/miniconda3/envs/experimental-all-conda/lib/python3.10/site-packages/myproxy/client/__init__.py", line 42, in <module>
    from OpenSSL import crypto, SSL
  File "/home/valeriu/miniconda3/envs/experimental-all-conda/lib/python3.10/site-packages/OpenSSL/__init__.py", line 8, in <module>
    from OpenSSL import crypto, SSL
  File "/home/valeriu/miniconda3/envs/experimental-all-conda/lib/python3.10/site-packages/OpenSSL/crypto.py", line 11, in <module>
    from OpenSSL._util import (
  File "/home/valeriu/miniconda3/envs/experimental-all-conda/lib/python3.10/site-packages/OpenSSL/_util.py", line 5, in <module>
    from cryptography.hazmat.bindings.openssl.binding import Binding
  File "/home/valeriu/miniconda3/envs/experimental-all-conda/lib/python3.10/site-packages/cryptography/hazmat/bindings/openssl/binding.py", line 14, in <module>
    from cryptography.hazmat.bindings._openssl import ffi, lib
ImportError: libssl.so.1.1: cannot open shared object file: No such file or directory

@zklaus
Copy link

zklaus commented Mar 17, 2022

@valeriupredoi, if pollution from the main channel is the problem, this indicates a problem in your local installation because the main channel shouldn't even be known to your local conda/mamba.

But the best mitigation might be the addition of the - nodefaults channel to the channel list of the environment.yml to prevent any packages from there.

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Mar 17, 2022

cheers @zklaus - are you 100% positive that adding nodefaults will grab everything from conda-forge? Coz that's a way more elegant solution than adding secondary deps in environment.yml - but I had heard stories that that works but not always?

@zklaus
Copy link

zklaus commented Mar 17, 2022

It's documented here for conda and definitely works for mamba.

@valeriupredoi
Copy link
Contributor Author

sweet! it's so goin in here then 🍺

…defaults channel, cheers to Klaus for suggesting it
@valeriupredoi valeriupredoi added this to the v2.6.0 milestone Mar 17, 2022
@valeriupredoi
Copy link
Contributor Author

@valeriupredoi valeriupredoi marked this pull request as ready for review March 17, 2022 15:28
@valeriupredoi

This comment was marked as outdated.

Co-authored-by: valeriupredoi <valeriupredoi@users.noreply.github.com>
Comment on lines -32 to -61
'install': [
'cartopy',
'cf-units>=3.0.0',
'dask[array]',
'esgf-pyclient>=0.3.1',
'esmpy!=8.1.0', # see github.com/ESMValGroup/ESMValCore/issues/1208
'fiona',
'fire',
'geopy',
'humanfriendly',
"importlib_resources;python_version<'3.9'",
'isodate',
'jinja2',
'nc-time-axis', # needed by iris.plot
'nested-lookup',
'netCDF4',
'numpy',
'pandas',
'pillow',
'prov',
'psutil',
'pybtex',
'pyyaml',
'requests',
'scipy>=1.6',
'scitools-iris>=3.1.0,<3.2.0',
'shapely[vectorized]',
'stratify',
'yamale',
],
Copy link

Choose a reason for hiding this comment

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

I think removing this stuff here is a mistake. The setup.py should absolutely have all the dependency information. This will be crucial to guarantee a correct installation when someone chooses an alternate method of installation. But it should be duplicated in the environment.yml file. That way, using conda/mamba is one way to install all the dependencies, using apt/yum/whatever is another, and on pip install . pip can verify that everything is in place as it should be.

Copy link
Contributor Author

@valeriupredoi valeriupredoi Mar 18, 2022

Choose a reason for hiding this comment

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

@zklaus this is a very good point and I also thought about it but I think we need to remove them because:

  • we don't want to run into those odd cases both you and I have seen when perfectly conda-forgeable dependencies were installed from PyPi instead (at the pip install stage)
  • we don't want to maintain two separate lists that may diverge - note that quite a few of the dependencies are not on PyPi but are only on conda-forge; there are a few that have slightly different names and versions PyPi vs conda-forge too
  • apt/yum works installations works only for the packages that are on Synaptic repo/RPM's, there are again those that are only on conda-forge (is there a way to install a purely conda-forge package with eg apt?)

BUT you do make a very good point about pip install -e . which is a good safeguard and test that all packages have been installed correctly (bar the wiggle from my first bullet point up). What if we read environment.yml in setup.py and checked the packages are indeed installed correctly and if not, prompt the installer to try again with mamba? Or, can you think of a better way to check at pip install stage?

Copy link

Choose a reason for hiding this comment

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

Here is how I think about this; perhaps we need to have a conversation in the technical lead team.

The source distribution on PyPI is not just another way to install things. It is the release. This is our product, it must contain correct information about dependencies, but it makes no claim about how to install the dependencies.

On conda-forge, there is a package of the PyPI release. Coincidentally, it happens to be produced by some of the same people that are working on the project as well. Similar thinking applies to modules. If we manage to straighten out our dependency handling, it is imaginable that further install methods be added in the future. Why not an ESMValTool package in debian+derivatives like ubuntu? Why not in Spack? All of these projects, including conda-forge, will take the information about dependencies from the PyPI source distribution.

Now, can we do something to reduce duplication? Perhaps. One thing we should probably consider is moving to an essentially empty setup.py with all of the information migrated to setup.cfg, as Iris did, following the official encouragement of the Python packaging crowd. Also, perhaps additionally, we could look at how other projects handle this, see, e.g., Sparse.

One tricky bit is that project names on PyPI and conda-forge sometimes differ. Perhaps the best way is to be clear that both lists must be maintained, to come up with a good way of handling the extras (test, doc, ...), and to simplify things as much as possible by avoiding dependencies of dependencies and having clear rules on what to include.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all very good points, Klaus! I have re-converted the PR to a Draft, I will reinstate our setup.py as it was before I made the changes, and I support the idea we talk this at the next @ESMValGroup/technical-lead-development-team technical meeting which I will be circulating a Doodle poll for next week 👍

Choose a reason for hiding this comment

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

Two cents from an outsider (although I've done packaging a bit), but maybe this conversation is already finished (in which case I'd be interested in the solution).

I think you basically just have to maintain multiple lists. As far as I can tell, it's impossible to only have a single source of truth (e.g. I have no idea how you could auto-compile your conda dependencies for the conda forge feedstock repo from the dependencies in your source repository). That's annoying, but I think just means you end up relying on really good testing (test it can be installed from conda, test it can be installed from pypi, test it can be installed from some other source etc.).

Copy link

Choose a reason for hiding this comment

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

I think @znicholls is mostly right, though the conda-forge bot helps us part of the way.

@valeriupredoi valeriupredoi marked this pull request as draft March 18, 2022 13:26
@sloosvel sloosvel modified the milestones: v2.6.0, v2.7.0 Jun 7, 2022
@valeriupredoi valeriupredoi modified the milestones: v2.7.0, v2.8.0 Oct 3, 2022
@valeriupredoi
Copy link
Contributor Author

bumping this to Milestone 2.8 since I am more than booked up with other stuff this week, including Milestone 2.7/freeze

@valeriupredoi
Copy link
Contributor Author

superseded by #1786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request installation Installation problem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install all dependecies from conda-forge

4 participants