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

Newest aiidateam/aiida-core:stable docker image contains broken pymatgen dependency #4339

Closed
greschd opened this issue Aug 28, 2020 · 37 comments · Fixed by #4353
Closed

Newest aiidateam/aiida-core:stable docker image contains broken pymatgen dependency #4339

greschd opened this issue Aug 28, 2020 · 37 comments · Fixed by #4353

Comments

@greschd
Copy link
Member

greschd commented Aug 28, 2020

Short description

The newest aiidateam/aiida-core:stable docker image contains pymatgen, but that is missing its dependency ruamel.yaml.

Update: The 1.3.0.1 image is also broken, while 1.3.0 works. I suspect it's something about these being built more recently - the latest pymatgen release is from 2020-08-13.

Steps to reproduce

  • docker pull aiidateam/aiida-core:stable
  • docker run aiidateam/aiida-core:stable
  • docker exec --tty $DOCKERID wait-for-services
  • docker exec -it $DOCKERID bash
  • su aiida
  • verdi shell
  • import pymatgen

Result:

(base) aiida@3e0b99c6409f:~$ verdi shell
Python 3.7.4 (default, Aug 13 2019, 20:35:49) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.17.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import pymatgen
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-1-a3a39e60e980> in <module>
----> 1 import pymatgen

/opt/conda/lib/python3.7/site-packages/pymatgen/__init__.py in <module>
     12 from fnmatch import fnmatch
     13 
---> 14 import ruamel.yaml as yaml
     15 
     16 from monty.json import MontyEncoder, MontyDecoder, MSONable

ModuleNotFoundError: No module named 'ruamel'

Expected behavior

The last command should import pymatgen.

Your environment

  • Docker version 19.03.12, build 48a66213fe
  • aiida-core version: 1.3.1
  • Python version: 3.7.4

Tagging @csadorf @yakutovicha

@greschd greschd added type/bug dependencies Pull requests that update a dependency file labels Aug 28, 2020
@csadorf
Copy link
Contributor

csadorf commented Aug 31, 2020

@greschd Thank you for reporting!
@yakutovicha Could you investigate, please?

@yakutovicha
Copy link
Contributor

Thanks @greschd. I will have a look!

@yakutovicha yakutovicha self-assigned this Aug 31, 2020
@csadorf csadorf added topic/dependencies and removed dependencies Pull requests that update a dependency file labels Aug 31, 2020
@yakutovicha
Copy link
Contributor

I am investigating this issue, and what I can tell already - when downgrading the pymatgen version the problem remains.

@yakutovicha
Copy link
Contributor

yakutovicha commented Sep 2, 2020

The content of /opt/conda/lib/python3.7/site-packages in the broken image:

ruamel_yaml/                   ruamel_yaml-0.15.80.dist-info/

Of the working (1.3.0) image:

ruamel/   ruamel_yaml/  ruamel_yaml-0.15.80.dist-info/  ruamel.yaml-0.16.10.dist-info/ 
ruamel.yaml-0.16.10-py3.8-nspkg.pth  ruamel.yaml.clib-0.2.0.dist-info/

So for some reason, the aiida-core==1.3.0 image installs the latest version of ruamel.yaml, while the newer version doesn't.

@yakutovicha
Copy link
Contributor

To temporarily fix the problem it is enough to do:

pip install --upgrade ruamel.yaml

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Yeah, it's weird that it needs to be pip installed though. AFAICT, the docker image uses conda in general, right?

@yakutovicha
Copy link
Contributor

Yeah, it's weird that it needs to be pip installed though. AFAICT, the docker image uses conda in general, right?

We use it mostly for the environment, for the moment. Many plugins are not available through conda

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Ah, maybe that's the source of the problem? If either pymatgen or ruamel.yaml are installed differently, maybe that could cause issues.

@yakutovicha
Copy link
Contributor

So for some reason, in old image build the log looked as follows:

Collecting ruamel.yaml>=0.15.6
Downloading ruamel.yaml-0.16.10-py2.py3-none-any.whl (111 kB)

Recently, it has been changed:

Requirement already satisfied: ruamel.yaml>=0.15.6 in /opt/conda/lib/python3.7/site-packages (from pymatgen>=2019.7.2; extra == "atomic_tools"->aiida-core==1.3.1) (0.15.80)

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Maybe some other dependency started requiring ruamel.yaml? I think that one is conda - installed, which may be incompatible with a pip - installed pymatgen for some reason?

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

I did try installing pymatgen onto a clean conda env (via conda), and that worked fine.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

I'm really not a conda expert, but this link seems relevant: https://www.anaconda.com/blog/using-pip-in-a-conda-environment

@yakutovicha
Copy link
Contributor

yakutovicha commented Sep 2, 2020

I did try installing pymatgen onto a clean conda env (via conda), and that worked fine.

yes, I've tried that as well, but via pip (being in a conda environment). Also worked fine.

@yakutovicha
Copy link
Contributor

ah, I might have a clue. The base image (aiidateam/aiida-prerequisites:0.2.0) has ruamel pre-installed

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Alright, here's a minimal repro (in a clean conda env, conda 4.8.4 on Ubuntu 18.04):

  • conda install ruamel_yaml
  • pip install pymatgen
  • python -> import pymatgen

It seems indeed the conda and pip installs are incompatible in this case.

@yakutovicha
Copy link
Contributor

Hmmmm, the old image did also have ruamel pre-installed as the dependency of miniconda:

The following packages will be downloaded:
package | build
---------------------------|-----------------
_libgcc_mutex-0.1 | conda_forge 3 KB conda-forge
_openmp_mutex-4.5 | 0_gnu 435 KB conda-forge
cffi-1.13.2 | py37h8022711_0 220 KB conda-forge
chardet-3.0.4 | py37_1003 167 KB conda-forge
conda-package-handling-1.6.0| py37h516909a_1 942 KB conda-forge
cryptography-2.8 | py37h72c5cf5_1 625 KB conda-forge
idna-2.9 | py_1 52 KB conda-forge
libffi-3.2.1 | he1b5a44_1006 46 KB conda-forge
libgcc-ng-9.2.0 | h24d8f2e_2 8.2 MB conda-forge
libgomp-9.2.0 | h24d8f2e_2 816 KB conda-forge
libstdcxx-ng-9.2.0 | hdf63c60_2 4.5 MB conda-forge
ncurses-6.1 | hf484d3e_1002 1.3 MB conda-forge
pycosat-0.6.3 |py37h516909a_1002 105 KB conda-forge
pycparser-2.19 | py_2 88 KB conda-forge
pyopenssl-19.1.0 | py_1 47 KB conda-forge
pysocks-1.7.1 | py37_0 27 KB conda-forge
readline-7.0 | hf8c457e_1001 391 KB conda-forge
requests-2.23.0 | py37_0 85 KB conda-forge
ruamel_yaml-0.15.80 |py37h516909a_1000 268 KB conda-forge
setuptools-45.2.0 | py37_0 654 KB conda-forge
six-1.14.0 | py37_0 23 KB conda-forge
sqlite-3.31.1 | h7b6447c_0 1.1 MB defaults
tk-8.6.10 | hed695b0_0 3.2 MB conda-forge
tqdm-4.43.0 | py_0 47 KB conda-forge
urllib3-1.25.7 | py37_0 159 KB conda-forge
wheel-0.34.2 | py_1 24 KB conda-forge
xz-5.2.4 | h14c3975_1001 366 KB conda-forge
yaml-0.2.2 | h516909a_1 82 KB conda-forge
zlib-1.2.11 | h516909a_1006 105 KB conda-forge

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Maybe related, even though it's old: materialsproject/pymatgen#758

@yakutovicha
Copy link
Contributor

My second guess is: we have different pip versions, and new pip might handle dependencies differently

@yakutovicha
Copy link
Contributor

yakutovicha commented Sep 2, 2020

Maybe related, even though it's old: materialsproject/pymatgen#758

I think this is not a problem. Ruaml has been put as a dependency quite some time ago, and we have working images build with it.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Alright, here's a minimal repro (in a clean conda env, conda 4.8.4 on Ubuntu 18.04):

* `conda install ruamel_yaml`

* `pip install pymatgen`

* `python` -> `import pymatgen`

It seems indeed the conda and pip installs are incompatible in this case.

Does this work on the old aiida-prerequisites container?

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

So.. conda has packages ruamel, ruamel_yaml and ruamel.yaml. Installing only ruamel_yaml does not work, but installing ruamel.yaml does.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

When installing ruamel_yaml, ... import ruamel_yaml works, but not import ruamel.yaml. For some reason pip still considers that a valid version for ruamel.yaml.

@yakutovicha
Copy link
Contributor

so looks like we should install ruamel.yaml==0.16.10 in the aiida-prerequisites image, what do you think?

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Yeah, I think that makes sense. TBH, I have no idea at all why there are two different conda-force sources for that.

@yakutovicha
Copy link
Contributor

I can now confirm that this is the pip problem. I downgraded pip and rebuild the image. It worked: ruamel.yaml got installed.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Even without the pip problem though, conda install ruamel_yaml doesn't work as advertised. I think pip installing a newer version on top is just masking the problem.

@yakutovicha
Copy link
Contributor

Even without the pip problem though, conda install ruamel_yaml doesn't work as advertised. I think pip installing a newer version on top is just masking the problem.

I agree.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

So, the two are actually different packages (ever so slightly): conda-forge/ruamel.yaml-feedstock#7 (comment)

Not sure who exactly is responsible for the fact that pip doesn't realize that, though..

@yakutovicha
Copy link
Contributor

So, the two are actually different packages (ever so slightly): conda-forge/ruamel.yaml-feedstock#7 (comment)

Not sure who exactly is responsible for the fact that pip doesn't realize that, though..

ah, my understanding was that it is the same package that just changed its name. But if those two packages are the different ones, we come back to the pip problem.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Yeah, so newer pip considers all of ruamel_yaml, ruamel-yaml and ruamel.yaml to be the same package..

The first two are fine (this is normal canonicalization), not sure what the standards say about the latter.

@yakutovicha
Copy link
Contributor

should we report this to pip?

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Might be pypa/pip#8659

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

It seems to be on purpose: pypa/pip#8645 (comment)

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

According to this page, package names aren't even supposed to contain dots 🙈

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

That's only semi-official though.. it's the packaging guide, not the official Python doc. Tried to find a PEP that specifies this, but so far no luck.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Yeah, so that's definitely intentional on the side of pip and PyPI. When you upload a package with any of -, _, ., it ends up in the same page. The displayed name is actually chosen by whichever it was uploaded as last 😄

Obviously, conda appears to have a different policy here. But it might be worthwhile to check with them if that is intentional.

@greschd
Copy link
Member Author

greschd commented Sep 2, 2020

Ah, it was actually changed just recently: conda-forge/ruamel_yaml-feedstock#68

That's only the conda-forge version though, not the default.

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

Successfully merging a pull request may close this issue.

3 participants