Insufficient ruamel.yaml dependencies on older systems

[ischoegl]

System information

• Cantera version: 2.5.0a4
• OS: Ubuntu Xenial with default packages
• Python/MATLAB version: 3.5.2

Expected behavior

scons test works out of the box, after successful scons build

Actual behavior

Test fails

Traceback (most recent call last):
  File "/src/test/python/runCythonTests.py", line 35, in <module>
    from cantera.test.utilities import unittest
  File "/src/build/python/cantera/test/__init__.py", line 13, in <module>
    from .test_convert import *
  File "/src/build/python/cantera/test/test_convert.py", line 8, in <module>
    from cantera import ck2cti, ck2yaml, cti2yaml, ctml2yaml
  File "/src/build/python/cantera/ck2yaml.py", line 100, in <module>
    yaml.RoundTripRepresenter.add_representer(float, represent_float)
AttributeError: module 'ruamel.yaml' has no attribute 'RoundTripRepresenter'

To Reproduce

Steps to reproduce the behavior:

1. Install Xenial (ubuntu 16.04 - same as current Travis) and update apt-get update
2. Install default ruamel-yaml via apt-get install python3-ruamel.yaml (which has version '0.10.23')
3. See above failure

Additional context

The alternate pip3 install ruamel-yaml works (version '0.16.10') ...

To resolve, a minimum version for ruamel.yaml appears to be necessary.

[bryanwweber]

I think the version check will have to go in ck2yaml.py, cti2yaml.py, and ctml2yaml.py. I wonder if that can somehow be centrally managed... Probably not

[ischoegl]

So there's no way to check for this in SConstruct? (I'm unfortunately not sufficiently familiar with the build process)

[bryanwweber]

There is, it just wouldn't be terribly useful since the same error would show up if a user installs an old version of ruamel.yaml with Pip or conda for some reason, and doesn't go through the build process to find the error. Certainly the conda package can specify the correct dependency but other installers can't.

[speth]

I don't see a reason not to check in SConstruct, though. That would be consistent with what we do for almost every other dependency. I don't think we should do checks in the individual converter scripts.

[bryanwweber]

What if a user installs the package from the PPA and installs ruamel.yaml from apt as well? Or for the Windows installer, they either already have installed or install new an old version of ruamel.yaml? They would run into the same error, but wouldn't get a useful error message to fix it. The same issue is likely to occur if we provide an RPM package for something like CentOS.

I guess one option is to not support the PPA on 16.04, and only support building from source, if we don't want to add the check to the scripts. I think the second problem is much less likely.

I would note that we check the NumPy version in these scripts though.

I'm not opposed too adding the check to Sconstruct, but I don't think that's enough.

[speth]

The Ubuntu package, and any other package for a packaging system that allows dependencies to be expressed, should specify an appropriate minimum version of ruamel.yaml, whatever that is (I don't think we've worked it out beyond what Ingmar's determined that that it lies somewhere between 0.10.23 and 0.16.10). I think this is somewhat independent of what checks the SCons build system does.

We may not be able to provide Ubuntu packages for 16.04, unless we also include an updated version of ruamel.yaml in the PPA.

[ischoegl]

fwiw ... .travis.yml specifies the version as 0.15.94 (although I'm not sure that this is the actual minimum version)

[bryanwweber]

a packaging system that allows dependencies to be expressed

At least the Windows installer doesn't allow this to be expressed. We have data for this now! About 30% of the people who responded to the survey used the Windows installer. Those people are also likely to have used pip to install ruamel.yaml, so this dependency isn't that likely to be a problem, but still...

I guess the question I have is, what's the harm of doing it in the scripts? Just that it has to go in 3 places? It would be equivalent of doing it in SConstruct because it would get checked at test time, but also would be useful for the installed interface.

[bryanwweber]

Coming back to this, I think it would be better to wait for user reports of errors before doing anything. The only platform that we currently support where this is somewhat likely to be a problem (IMO) is Ubuntu 16.04, which goes out of support next year, and even then it's only a problem if you're using the system Python with ruamel.yaml installed by apt. On Windows, people will be using pip to install ruamel.yaml, which will install a sufficiently new version.

[ischoegl]

For many build systems, it is customary to check for minimum versions of dependencies during compilation (at least this was the original intent of this issue report). It obviously won't apply for pre-packaged installs, but it should be straight-forward to check when using scons?

[bryanwweber]

Do we have any idea what the actual minimum version is? Does it matter if we get to the exact minimum version? I just tried to look, and I couldn't find it in 20 minutes or so. As far back as I can see, the RoundTripRepresenter class is present. One problem is that he uses Mercurial for version control, and I'm not that familiar with how to use hg...

If we can arbitrarily pick 0.15.94 (since we know that works and its from April 2019, so fairly old), then that's easy enough to check for in SConstruct.

[ischoegl]

We don’t, but 0.15.94 sounds reasonable. On a related note, would it make sense to create a requirements.txt file that holds dependencies checked by pip during installation?

[speth]

Maybe I'm missing something, but how would that work? Cantera can't currently be installed using pip.

[ischoegl]

That is true, but it's likely also true that scons can call pip to check pre-req's (via Execute?). That way all the dependency checks are outsourced.

PS: I can throw a PR together ... which shouldn't be more than a couple of lines.

Nevermind. I think there are probably use cases where people want to install without admin rights, so this may not be workable.

[bryanwweber]

I don't think we should assume that pip is installed on the user's computer. I'm also not sure how pip would check the dependencies... A check similar to the NumPy and Cython checks should be sufficient. I'll take care of it later today/tomorrow.

[ischoegl]

@bryanwweber ... fair point about pip (which can be used without root privileges after all). I thought it would still be worth bringing up.

FWIW, this would be the procedure:

Create requirements.txt

cython >= 0.23; python_version >= "3.5" and python_version < "3.8"
cython >= 0.29.12; python_version >= "3.8"
numpy >= 1.8.1
ruamel.yaml >= 0.15.34

and run:

(base) PS C:\Users\ischo\GitHub\cantera> pip install -r .\requirements.txt
Ignoring cython: markers 'python_version >= "3.8"' don't match your environment
Requirement already satisfied: numpy>=1.8.1 in c:\users\ischo\anaconda3\lib\site-packages (from -r .\requirements.txt (line 2)) (1.18.1)
Requirement already satisfied: cython>=0.23 in c:\users\ischo\anaconda3\lib\site-packages (from -r .\requirements.txt (line 3)) (0.29.17)
Requirement already satisfied: ruamel_yaml>=0.15.34 in c:\users\ischo\anaconda3\lib\site-packages (from -r .\requirements.txt (line 4)) (0.15.87)

PS: I used 0.15.87 as this is what the base version of my anaconda3 installation was 0.15.34 as this is the status of Ubuntu 18.04.

PPS: What I had in mind was to put Execute('pip install -r ./requirements.txt') into SConstruct.

[bryanwweber]

@ischoegl Thanks, I can see how that would work now. I think it's better just to have the minimum version check in #891. As you point out, it will have to be 0.15.87 which is available from conda.

[ischoegl]

@bryanwweber ... np - whatever you do is fine ... although the pip route would shorten the SConstruct quite a bit (note that the lines above also include the conditional cython version) 😉