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

ck2yaml and cti2yaml require Python 3, breaking builds that use system Python 2 #669

Closed
bryanwweber opened this issue Jul 8, 2019 · 7 comments

Comments

@bryanwweber
Copy link
Member

Cantera version

2.5.0a3

Operating System

Any

When building from source, either Python 2 or Python 3 should be able to be used to run SCons. The new ck2yaml and cti2yaml scripts depend on Python 3 features. The build script relies on at least ck2yaml to convert CHEMKIN input files to the YAML format, so builds that use Python 2 to run SCons and don't specify a python_cmd that is a Python 3 executable result in errors. One of them is documented on the Google Group:

scons: Building targets ...
"/usr/bin/python" interfaces/cython/cantera/ck2yaml.py --quiet --no-validate --input=data/inputs/gri30.inp --output=build/data/gri30.yaml --transport=data/transport/gri30_tran.dat
  File "interfaces/cython/cantera/ck2yaml.py", line 177
    def __init__(self, *, Tmin, Tmax, Tmid, low_coeffs, high_coeffs, note=''):
                        ^
SyntaxError: invalid syntax
scons: *** [build/data/gri30.yaml] Error 1
scons: building terminated because of errors.

Steps to reproduce

  1. Run SCons with a Python 2 sys.executable and do not specify a Python 3 executable in python_cmd

One possible solution to this is to check the YAML files into the repository rather than creating them during the build process. Another option is to make the scripts compatible with Python 2.

@ischoegl
Copy link
Member

ischoegl commented Aug 5, 2019

Python 2 EOL is January 1st, 2020. What is cantera's perspective for Python 2 support beyond this date?

@bryanwweber
Copy link
Member Author

For the Python interface, 2.4.x will be the last release to support Python 2. We are attempting to keep the build script Python 2 compatible for the foreseeable future, so that, e.g., apt install scons and scons build will "just work". It's still under discussion, but I'm pretty sure the fix to this issue and #662 is going to be to check the YAML input files into the repository. Before we do that, I'm working on a CTML->YAML converter.

@ischoegl
Copy link
Member

ischoegl commented Aug 5, 2019

@bryanwweber ... thanks for the clarification. From a user perspective, being forced to consistently use Python 3 may have some advantages: a meaningful error message would avoid quite a bit of confusion (e.g. user forum questions). From what I can gather, a recent scons version is all that is required?

@bryanwweber
Copy link
Member Author

I'm not sure what you're asking... Basically, since the default system Python is likely to be Python 2 on many of the platforms we support, and we want SCons to be able to use that Python, we are trying to keep the build process Python 2 compatible. However, the *2yaml scripts require Python 3 as noted here. Thus, if the YAML files are checked into the repository, the conversion won't be necessary and we can maintain Python 2 compatibility.

@ischoegl
Copy link
Member

ischoegl commented Aug 6, 2019

The question was probably rhetorical. It boils down to eliminating a choice of python_cmd defaulting to Python 2 for the scons configuration, i.e.

* python_cmd: [ /path/to/python_cmd ]
    Cantera needs to know where to find the Python interpreter. If
    PYTHON_CMD is not set, then the configuration process will use the
    same Python interpreter being used by SCons.
    - default: '/usr/bin/python'
    - actual: 'python3'

which instead should simply enforce a Python 3 interpreter. This would cut down on the periodic question/confusion on the user forum, and would presumably make maintenance easier (this issue is a case in point). Imho there isn't much of a point in prolonging the scheduled obsolescence of Python 2 more than necessary ... of course those are just my 2 cents.

@bryanwweber
Copy link
Member Author

OK, I see what you're saying. I don't remember whether or not the check for python_cmd enforces Python 3, but it probably should if it doesn't. And Python 2 only being present shouldn't trigger building the Python interface.

Regardless, I don't think it makes sense for users to have to install Python 3 on their systems, since we want to support building just the C/C++/Fortran/MATLAB interfaces with the system Python (i.e., mostly Python 2) running SCons. This means we can't rely on Python 3 being available to run the *2yaml scripts. Plus, the *2yaml scripts have an additional dependency on ruamel.yaml, which would be required to be present during the build process (that's what's reported in #662).

@bryanwweber
Copy link
Member Author

This should be resolved by #768

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

No branches or pull requests

2 participants