Add package workflow

[bryanwweber]

Changes proposed in this pull request

• Trigger workflows in the Conda, PyPI, Windows, and macOS packaging repositories. The workflows are triggered on commits to main in this repo and to tags pushed to this repo.
• Change the Python interfaces to use pip as the builder and installer

Checklist

• The pull request includes a clear description of this code change
• Commit messages have short titles and reference relevant issues
• Build passes (scons build & scons test) and unit tests address code coverage
• Style & formatting of contributed code follows contributing guidelines
• The pull request is ready for review

[bryanwweber]

@Cantera/committers I'd appreciate some eyes on this PR, as I'm getting into rewriting the Python installer logic in the SConscript. I added it to this branch because I'm hoping to use the rewritten Python installer to fix the Conda packages, and then add a job here to run the Conda builds when a PR is merged to main. Note the change to .github/workflow/main.yml to disable it on PRs is temporary for debugging here, I'll remove that change. I didn't want to run a zillion builds of Cantera when I wasn't changing any files that are part of the build 😄

See the details section 👇 for some justification/explanation of the changes:

Click for more

1. I elected to use a VariantDir with duplicate=True for the Cython interface. This copies all the dependent files into the build/python directory, so all the building commands can be run against that single directory, for example, pip wheel build/python works now to create a wheel.
2. The change to a VariantDir also means that setup.py build is no longer required, as far as I can tell. Since all the files are put into the build/python directory by SCons, the same trick of PYTHONPATH=build/python ... still works as it always has. There are also no longer any built files put into the interfaces/cython directory, which is a nice bonus for cleanup (although I haven't changed the scons clean code yet).
3. I opted to use pip as the build frontend to install the Python package. This is because as of Setuptools 60.0.0, using setup.py install is deprecated in favor of the explicit frontend/backend separation specified in PEP 517/518. In this sense, a frontend tells the backend what to build (sdist, wheel, egg, etc.) while the backend actually creates that resource. Our backend is setuptools (and has been for a long time); prior to this change, we were also using setuptools as the frontend via setup.py install. As this behavior is now deprecated, I opted for pip as the more likely candidate to be installed on someone's machine than build (which is used to generate the sdist via scons sdist). This means that pip install build/python should also work as expected.

FYI @speth, as we discussed in Cantera/enhancements#129, this branch has an example of triggering a workflow in another repo. You made a good point that it'd be nice if we can somehow drag the status of the triggered workflow back into the status of this repo. I'm sure it's possible, I'll see how to do it. Worst case, it will be a "badge" on the README with the workflow status in the other repositories.

[ischoegl]

👀 ... (kidding)

I am aware of what you pointed out about (3) ... have been using the pip route for a while elsewhere, so this is absolutely needed! Beyond, I'm probably not super-familiar with the SCons specific changes (sorry), but the direction sounds great!

Triggering Conda builds upon PR merges sounds terrific! 🎉 ... this comes very close to Cantera/enhancements#24 (which is superseded by Cantera/enhancements#37) and should also eliminate issues like Cantera/conda-recipes#24 (haven't checked recently whether this has since been resolved).

[bryanwweber]

But what other directory even makes sense to install to, if you're using /usr/bin/python3 and installing the rest of Cantera to /usr/local? If you're following pip's recommendation to use virtual environments, then python3 should be a location within a venv or conda environment. By this logic, should the build just fail if python_cmd isn't set explicitly but evaluates to /usr/bin/python3? Why wait for the install step?

@speth / @ischoegl OK, after reflecting this week, I think enforcing installation directories is probably a step too far. The justification for it isn't quite as strong as I thought, and you've both raised some really good points. In the end, I think I'll defer improved error messages from this PR to a later one, and just focus this one on using pip as the Python installer so the Conda packages can work again. I'm going to mark a bunch of the comments in this thread as off topic to reduce the clutter as this PR moves towards review.

[speth]

I'll admit that I understand approximately nothing about how Python packaging is supposed to work anymore, but I don't see any major problems with this, and it seems to work correctly for the couple of different configurations I tried.

[speth]

First, I think if you define the MACOSX_DEPLOYMENT_TARGET environment variable, you don't need to specify the compiler option. If you want to use the compiler option instead, though, it should also be set for CCFLAGS so it will affect pure C code like SUNDIALS.

Second, while I'll admit this is as clear as mud to me, I'm not sure it's necessary for libraries that get linked together to have the same OS target version. The conda-forge maintainer docs do have a bit to say about this (https://conda-forge.org/docs/maintainer/knowledge_base.html#requiring-newer-macos-sdks), which mostly boils down to saying that they compile for 10.9 to maintain compatibility with macOS 10.9, but suggests that requiring a higher version is something that can be done.

Certainly, when building packages that we distribute, we should be setting MACOSX_DEPLOYMENT_TARGET to an older value than whatever it defaults to, and I think that should be done regardless of whether you're compiling the Python module (but again, I don't think anything needs to be written here to achieve that).

[speth]

I was sort of hoping we could move toward doing away with any copying of the Python module parts, since that would make it easier to do rapid iteration and interactive debugging of the pure-Python components.

[bryanwweber]

That's unfortunate... For me, it's so much easier to reason about actual files than abstract file nodes. I think it also streamlines the installation method (pip), which doesn't have to account for the built library being in a different location, and which also doesn't know anything about file nodes.

When possible, SCons hard links files rather than copying them. However, most editors (to my understanding) use a copy-then-move strategy to save files, which creates a new inode and breaks the hardlink, so that doesn't actually help us.

What I've done is use pytest-watch, which allows automatically running a command before running the test suite whenever a file is saved. This doesn't support the interactive feature development case so much, but it's better than nothing.

[bryanwweber]

@speth Do you have any further thoughts here?

[ischoegl]

I gave this a spin on both Linux (conda on Fedora) and Windows (likewise from within conda). Once I made sure to run scons clean, things worked as expected 😆

For windows conda with prefix (i.e. scons build -j4 prefix=C:\Users\ischo\cantera2.6.0a4, and scons install), I get

[...]
postInstallMessage(["finish_install"], [])

Cantera has been successfully installed.

File locations:

  library files               C:\Users\ischo\cantera2.6.0a4\lib
  C++ headers                 C:\Users\ischo\cantera2.6.0a4\include
  samples                     C:\Users\ischo\cantera2.6.0a4\samples
  data files                  C:\Users\ischo\cantera2.6.0a4\data
  input file converters       C:\Users\ischo\miniconda3\envs\cantera-dev\Scripts
  Python package              C:\Users\ischo\miniconda3\envs\cantera-dev\Lib\site-packages
  Python samples              C:\Users\ischo\miniconda3\envs\cantera-dev\Lib\site-packages\cantera\examples
scons: done building targets.

so the input file converter location is now displayed correctly. While it's not what I would have expected, it works and is user-friendly. As such, this closes #1192.

Uninstalling works in all cases (again, scons clean in between compilations is pretty important).

Running scons clean resolves the issue.

[ischoegl]

While I lack the background to really comment on the packaging aspect, I ran a couple of tests installing and everything works as expected. The install message newline and script location when prefix is specified is optional (presumably, the script location is dictated by python_prefix?).

Fwiw, this PR closes both #1192 and #1194.

[bryanwweber]

@ischoegl Thanks, I think I addressed your comments! It also helped me find the problem with the prefix and the new Python installer, so it was 100% worth doing 😄

[codecov]

Codecov Report

Merging #1158 (e680b8f) into main (719275e) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1158   +/-   ##
=======================================
  Coverage   65.43%   65.44%           
=======================================
  Files         320      327    +7     
  Lines       46249    46321   +72     
  Branches    19657    19688   +31     
=======================================
+ Hits        30265    30314   +49     
- Misses      13454    13475   +21     
- Partials     2530     2532    +2     

Impacted Files	Coverage Δ
src/kinetics/RxnRates.cpp	78.26% <0.00%> (-14.47%)	⬇️
include/cantera/kinetics/ReactionData.h	76.00% <0.00%> (-12.89%)	⬇️
include/cantera/kinetics/Arrhenius.h	70.90% <0.00%> (-11.57%)	⬇️
src/kinetics/InterfaceKinetics.cpp	72.23% <0.00%> (-3.56%)	⬇️
include/cantera/kinetics/RxnRates.h	90.90% <0.00%> (-2.43%)	⬇️
include/cantera/kinetics/InterfaceRate.h	82.73% <0.00%> (-1.41%)	⬇️
src/kinetics/InterfaceRate.cpp	91.07% <0.00%> (-0.75%)	⬇️
src/kinetics/Arrhenius.cpp	98.46% <0.00%> (-0.53%)	⬇️
src/kinetics/Kinetics.cpp	68.33% <0.00%> (-0.48%)	⬇️
src/thermo/MixtureFugacityTP.cpp	37.03% <0.00%> (-0.47%)	⬇️
... and 29 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[ischoegl]

Thanks! Tried on windows with prefix again, but something apparently broke:

(cantera-dev) PS C:\Users\ischo\GitHub\cantera> scons build -j4 prefix=C:\Users\ischo\cantera2.6.0a4
scons: Reading SConscript files ...
INFO: SCons is using the following Python interpreter: C:\Users\ischo\miniconda3\envs\cantera-dev\python.exe
Compiling with MSVC 14.3
Compiling for architecture: amd64
Compiling using the following toolchain(s): ['default']
INFO: Building Cantera from git commit '8e1079a21'
INFO: Configuration variables read from 'cantera.conf' and command line:
    prefix = 'C:\\Users\\ischo\\cantera2.6.0a4'

INFO: Adding conda include and library paths: C:\Users\ischo\miniconda3\envs\cantera-dev
Checking for C++ header file cmath... yes
Checking for C++ header file fmt/ostream.h... no
INFO: Using private installation of fmt library.
INFO: Found fmt version 6.2.1
Checking for YAML::Node().Mark()... yes
INFO: Using system installation of yaml-cpp library.
Checking for C++ header file gtest/gtest.h... no
Checking for C++ header file gmock/gmock.h... no
INFO: Using Googletest from Git submodule
Checking for C++ header file eigen3/Eigen/Dense... yes
INFO: Using system installation of Eigen.
INFO: Found Eigen version 3.4.0
Checking whether __GLIBCXX__ is declared... no
Checking whether _LIBCPP_VERSION is declared... no
Checking whether __clang__ is declared... no
Checking for C++ library iomp5... no
Checking for C++ library omp... no
Checking for C++ library gomp... no
INFO: Found Boost version 1.77
Checking whether boost::core::demangle is declared... yes
Checking for CVodeCreate(CV_BDF, CV_NEWTON) in C++ library sundials_cvodes... no
Checking for CVodeCreate(CV_BDF) in C++ library sundials_cvodes... no
Checking for SUNContext ctx; SUNContext_Create(0, &ctx) in C++ library sundials_cvodes... no
Checking for double x; log(x) in C library None... yes
INFO: Using private installation of Sundials version 5.3.
INFO: Skipping compilation of the Fortran 90 interface.
INFO: Using NumPy version 1.22.2.
INFO: Using Cython version 0.29.28.
INFO: Building the full Python package for Python 3.10
  File "<string>", line 10
    prefix="C:\Users\ischo\cantera2.6.0a4")
                                          ^
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 2-3: truncated \UXXXXXXXX escape
CalledProcessError: Command '['C:\\Users\\ischo\\miniconda3\\envs\\cantera-dev\\python.exe', '-c', '\nfrom pip import __version__ as pip_version\nfrom pkg_resources import parse_version\nimport pip\nimport json\npip_version = parse_version(pip_version)\nif pip_version < parse_version("10.0.0"):\n    from pip.locations import distutils_scheme\n    scheme = distutils_scheme("Cantera", user=False, root=None,\n                              prefix="C:\\Users\\ischo\\cantera2.6.0a4")\nelse:\n    from pip._internal.locations import get_scheme\n    scheme = get_scheme("Cantera", user=False, root=None,\n                        prefix="C:\\Users\\ischo\\cantera2.6.0a4")\n\nif not isinstance(scheme, dict):\n    scheme = {k: getattr(scheme, k) for k in dir(scheme)\n               if not k.startswith("_")}\nprint(json.dumps(scheme))\n']' returned non-zero exit status 1.:
  File "C:\Users\ischo\GitHub\cantera\SConstruct", line 1964:
    SConscript("build/python/SConscript")
  File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\site-packages\SCons\Script\SConscript.py", line 660:
    return method(*args, **kw)
  File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\site-packages\SCons\Script\SConscript.py", line 597:
    return _SConscript(self.fs, *files, **subst_kw)
  File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\site-packages\SCons\Script\SConscript.py", line 285:
    exec(compile(scriptdata, scriptname, 'exec'), call_stack[-1].globals)
  File "C:\Users\ischo\GitHub\cantera\build\python\SConscript", line 154:
    install_locs = get_pip_install_location(localenv["python_cmd"], user_install,
  File "C:\Users\ischo\GitHub\cantera\site_scons\buildutils.py", line 1245:
    return json.loads(get_command_output(python_cmd, "-c", install_script))
  File "C:\Users\ischo\GitHub\cantera\site_scons\buildutils.py", line 1199:
    data = subprocess.run(
  File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\subprocess.py", line 524:
    raise CalledProcessError(retcode, process.args,
(cantera-dev) PS C:\Users\ischo\GitHub\cantera>

(I made sure to run scons clean)

[bryanwweber]

Thanks! Tried on windows with prefix again, but something apparently broke:

Yeah, this is the same failure as on the Windows CI. Won't be hard to fix 👍

[bryanwweber]

Thanks! Tried on windows with prefix again, but something apparently broke:

@ischoegl Ah, this is apparently a different error, actually, due to Windows insistence on using the wrong slash for a path separator. The paths should be normalized by that point, so I'm not sure what's going on, but I can reproduce locally and I'll fix it 👍

[ischoegl]

post-install message for scons build prefix=C:\Users\ischo\cantera2.6.0a4

[...]
postInstallMessage(["finish_install"], [])

    Cantera has been successfully installed.

    File locations:
      library files               C:\Users\ischo\cantera2.6.0a4\lib
      C++ headers                 C:\Users\ischo\cantera2.6.0a4\include
      samples                     C:/Users/ischo/cantera2.6.0a4\samples
      data files                  C:/Users/ischo/cantera2.6.0a4\data
      input file converters       C:\Users\ischo\cantera2.6.0a4\Scripts
      Python package              C:\Users\ischo\cantera2.6.0a4\Lib\site-packages
      Python examples             C:\Users\ischo\cantera2.6.0a4\Lib\site-packages\cantera\examples

scons: done building targets.

[ischoegl]

While the forward/backward slashes aren't pretty, this shouldn't hold back the beta release.