mods to enable python3.12

[mefuller]

Changes proposed in this pull request

• enable building Cantera with Python 3.12 (thanks for the pointer @speth)
• update docs in source to include Python 3.12
• verified in Fedora 39, Rawhide builds (e.g. https://koji.fedoraproject.org/koji/buildinfo?buildID=2279235)

Closes #1602

Checklist

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

[ischoegl]

Thanks, @mefuller! Too bad that Python 3.12 isn't available on the regular channels yet; it is definitely true that Python 3.12 needs to be covered by CI if we enable this.

There is one option I see (if we truly want to live on the bleeding edge), which is to use a fedora:rawhide container. The following GH action should work:

  fedora-runner:
    name: Fedora rawhide test
    runs-on: ubuntu-latest
    container:
      image: fedora:rawhide
    steps:
    - uses: actions/checkout@v3
    - name: Install dependencies
      run: |
        dnf -y install <all the cantera dependencies>
    ...

Other than adding this to .github/workflows/main.yaml, an alternative would be to just test in .github/workflows/post-merge-tests.yaml, which would be a great middle-ground. (PS: for review purposes, it should probably be added in main first, before moving it to post-merge-tests prior to merging)

PPS: see #1608, which implements the idea for Ubuntu containers

[bryanwweber]

3.12 release candidates are usually available from the setup-python action, if it helps. Since conda won't get binaries (and of dependencies) until the final release, everything will have to come from pip anyways.

[speth]

Other than adding this to .github/workflows/main.yaml, an alternative would be to just test in .github/workflows/post-merge-tests.yaml, which would be a great middle-ground. (PS: for review purposes, it should probably be added in main first, before moving it to post-merge-tests prior to merging)

I agree that this would be better in the post-merge-tests.yaml. I'm wondering if there's a way to modify this workflow to be able to manually run it on a PR / fork. My best guess is that we'd need to add another input to the workflow_dispatch section, and then modify the checkout action to specify that fork instead of defaulting to the base repository.

[ischoegl]

I agree that this would be better in the post-merge-tests.yaml. I'm wondering if there's a way to modify this workflow to be able to manually run it on a PR / fork. My best guess is that we'd need to add another input to the workflow_dispatch section, and then modify the checkout action to specify that fork instead of defaulting to the base repository.

I have a container-based CI run ~almost~ ready to go in #1608 (using ubuntu images; it's easy to go to different linux flavors from there). Unfortunately, I'm not familiar with manually-triggered runs though.

[ischoegl]

3.12 release candidates are usually available from the setup-python action, if it helps. Since conda won't get binaries (and of dependencies) until the final release, everything will have to come from pip anyways.

@bryanwweber ... it's unfortunately not available yet, which is why I suggested the Fedora runner. This btw also relates to Cantera/enhancements#37. PS: just found one of your posts mentioning deadsnakes, which would be an alternative.

[mefuller]

@ischoegl is there any additional information you'd need from me on running the build in Fedora? I'm happy to help

For reference, the RPM spec file (which generates the packaged binaries) is found at https://src.fedoraproject.org/rpms/cantera/blob/rawhide/f/cantera.spec

That file has a complete list of packages required for building and the exact arguments passed to scons

[bryanwweber]

@ischoegl The docs definitely mention allowing pre-release versions: https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md (all the way at the bottom, sorry I'm on my phone so I can't directly link). This should be orthogonal to adding container-based actions, though

[bryanwweber]

My best guess is that we'd need to add another input to the workflow_dispatch section, and then modify the checkout action to specify that fork instead of defaulting to the base repository.

If a PR is open for a branch, the merged commit (I believe this means "as though the PR branch was merged into the base branch") is available in the base repository without needing to specify the fork. I'd have to look up how to get that commit, though... 😕

[ischoegl]

@ischoegl The docs definitely mention allowing pre-release versions: https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md (all the way at the bottom, sorry I'm on my phone so I can't directly link). This should be orthogonal to adding container-based actions, though

Yes, it is orthogonal. @mefuller actually used the setup-python action in this PR, but the runner failed. I believe the critical information is on the docs you linked (the allow-prerelease: true bit) :tada:

      - uses: actions/setup-python@v4
        with:
          python-version: "${{ matrix.python_version }}"
          allow-prereleases: true

@mefuller ... I believe easiest path forward for this PR is to add the option to the setup-python action above; once 3.12 is covered by CI, the PR itself is ready to merge. I still believe it would make sense to add a post-merge-test for fedora eventually (although I won't do this myself: this is likely more of an interest to the fedora community). They are relatively easy to set up, as demonstrated in #1608.

[speth]

I don't think we should be testing any pre-release software as part of the main.yml workflow that runs on every update to a PR -- post-merge-tests.yml is a more appropriate place for such tests. In this case, such builds will be slower because they will need to compile dependencies (notably, numpy) from source. And in general, such builds are much more likely to have failures that have nothing to do with Cantera, and I think that's just an unnecessary source of noise when trying to evaluate a PR.

Given the release schedule for Python 3.12 (planned for October 2), that either means moving this job to the other workflow, or waiting until then to merge this PR.

In either case, I'm 👍 on adding a Rawhide container build to post-merge-tests.yml, though that's perhaps separate from this PR.

[ischoegl]

@ischoegl is there any additional information you'd need from me on running the build in Fedora? I'm happy to help

For reference, the RPM spec file (which generates the packaged binaries) is found at https://src.fedoraproject.org/rpms/cantera/blob/rawhide/f/cantera.spec

That file has a complete list of packages required for building and the exact arguments passed to scons

@mefuller ... now that #1608 is merged, could you rebase? Running tests in a Fedora container, - rawhide and/or latest - should be very simple to set up (likewise for Python, but I'm less of a Pythonista than @bryanwweber). There's only a couple of lines that need to be edited from the ubuntu version (see commit 9ee13eb). Let me know if you have questions, I'd be happy to help!

As an aside, I had a look over the dependencies; you may want to consider the new/optional libhdf5 dependency for your Fedora builds (Cantera 3.0 and later) as we deprecated/removed h5py support (Fedora likely doesn't have the odd install location that ubuntu is using); I also noticed that you're not explicitly listing any blas/lapack libraries (although the Cantera installer may find something that is implicit), which works but likely isn't the best option (for the Cantera conda channel; we use openblas on Linux systems). Other than that, scipy is only used for examples, i.e. optional.

PS: I created a runner here: 81c1058 although I have not tested whether it works. Feel free to cherry-pick!

[mefuller]

@ischoegl I made the requested/suggested changes with the exception of HDF5 HDF5 support is something I want to get into the Fedora Cantera build, but I need to get some other maintainers to update a few dependencies OpenBLAS is an implicit requirement since it's pulled in by other deps in the spec file (this is considered good form), but in the next Fedora build I will add system_blas_lapack=y to the scons build for good housekeeping

[ischoegl]

Thanks, @mefuller! This is almost there. I'd just have a few more requests:

1. could you remove the '3.12' from line 64 in main.yaml as indicated above? That way, we won't have failing tests.
2. temporarily copy fedora-docker over to main.yaml in a new commit, and push so CI runs a test. This is to see whether the job is configured correctly
3. once everything runs, remove the test from main.yaml again (by dropping the commit), so things are ready to merge

This will allow us to review the output of the run on the Actions tab. Ideally, all of the tests (both main and post-merge-tests) should run successfully at this point.

Regarding HDF5, it presumably will compile on fedora? Testing this is a question that is likely separate from packaging decisions, but I'll leave this up to you. Fwiw, the HighFive package we're using is header only, so it is no longer needed once things are compiled.

[ischoegl]

Thanks, @mefuller :tada: … tests look great! While Python 3.12 adds some syntax warnings for raw strings in SConstruct, this is not preventing compilation from failing and may go beyond this PR. I’ll approve from my side once the last commit is dropped.

[mefuller]

@ischoegl test commit dropped

[ischoegl]

@mefuller ... thanks again. Regarding hdf5, it looks like fedora actually packages highfive-devel, but it's rather dated (2 years old), see https://packages.fedoraproject.org/pkgs/highfive/highfive-devel/fedora-rawhide.html and https://github.com/BlueBrain/HighFive/releases/tag/v2.3.1 ... is this what you were referring to?

[mefuller]

@ischoegl exactly - I'm on it See https://bugzilla.redhat.com/show_bug.cgi?id=2236670