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

[scons] Bump python_max_p1_version to 3.13 #1648

Merged
merged 7 commits into from
Dec 12, 2023

Conversation

matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented Dec 4, 2023

Followup of #1604, #1612. See also conda-forge/cantera-feedstock#35 for context.

Changes proposed in this pull request

  • Allows building the package with Python 3.12

If applicable, fill in the issue number this pull request is fixing

If applicable, provide an example illustrating new features this pull request is introducing

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

@matthiasdiener matthiasdiener marked this pull request as ready for review December 4, 2023 21:09
@bryanwweber
Copy link
Member

I think we also need to bump the CI runner configuration to make sure we test on 3.12.

@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented Dec 6, 2023

I think we also need to bump the CI runner configuration to make sure we test on 3.12.

Thanks, I added 3.12 to the CI configs.

@bryanwweber
Copy link
Member

It looks like we might need to bump Cython to fix that build, but I'm not sure why it's only failing on macOS. Very strange.

@matthiasdiener
Copy link
Contributor Author

It looks like we might need to bump Cython to fix that build, but I'm not sure why it's only failing on macOS. Very strange.

As far as I can see, the cython version is the same across all builds (3.0.6). I believe it is just using an old setuptools version for the macos build, which I hopefully resolved in f0f7d28.

@matthiasdiener
Copy link
Contributor Author

That seems to have fixed it. The remaining CI errors look the same as in main, so I think this is ready to go.

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you would like to also fix the other CI errors, you can cherry pick 5128a89 from #1647.

.github/workflows/main.yml Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (60ea75a) 72.68% compared to head (880de60) 72.68%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1648      +/-   ##
==========================================
- Coverage   72.68%   72.68%   -0.01%     
==========================================
  Files         370      370              
  Lines       56298    56302       +4     
  Branches    20403    20403              
==========================================
+ Hits        40922    40923       +1     
- Misses      12369    12371       +2     
- Partials     3007     3008       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @matthiasdiener, this looks good to me.

@speth speth merged commit 87317b7 into Cantera:main Dec 12, 2023
46 of 49 checks passed
@matthiasdiener matthiasdiener deleted the scons-3.12 branch December 12, 2023 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants