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

Add Python 3.11 to GH Actions #1341

Merged
merged 14 commits into from
Aug 8, 2023
Merged

Add Python 3.11 to GH Actions #1341

merged 14 commits into from
Aug 8, 2023

Conversation

opotowsky
Copy link
Member

@opotowsky opotowsky commented Jul 10, 2023

What is the change?

Updating ARMI to run CI with python 3.11.

Note, however, the docs GH action is remaining behind with python 3.9.

Why is the change being made?

Python 3.11 offers a 10-40% speedup! Let's get with the program.

Resolves Issue #957, but Opens Issues #1344 and #1377


Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any important changes.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

@opotowsky
Copy link
Member Author

Just FYI there are a bunch of issues with the docs requirements that need updating. I can update all the packages to 3.11-compliant versions and hope that old bugs have been fixed that requires the version pin?

Or we could leave the GH action to build docs with python 3.9 like it does now. But someone wouldn't be able to build them locally if they were using 3.11. Thoughts @john-science?

@john-science
Copy link
Member

We can just continue to build the docs on GitHub with Python 3.9, and get the tests and downstream simulations up to 3.11. Right?

And transfer the docs later?

They are fairly independent.

@opotowsky
Copy link
Member Author

We can just continue to build the docs on GitHub with Python 3.9, and get the tests and downstream simulations up to 3.11. Right?

Yes I think it's fine to do that on CI. Someone building the docs locally will need to have a separate 3.9 venv, though. I've actually never built ARMI's locally I'm ashamed to say.

@opotowsky opotowsky linked an issue Jul 13, 2023 that may be closed by this pull request
setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@opotowsky opotowsky changed the title [WIP] Py311 Add Python 3.11 to GH Actions Jul 13, 2023
@opotowsky opotowsky marked this pull request as ready for review July 13, 2023 17:29
@opotowsky opotowsky requested a review from john-science July 14, 2023 16:04
@opotowsky
Copy link
Member Author

opotowsky commented Jul 14, 2023

@john-science I have a few questions above. I can't test mpi4py update with our internal code for a week or three but I'm worried about pinned version dependency clashes, so I'm proposing removing the version pin altogether (or could update downstream and test it on python 3.9).

@opotowsky
Copy link
Member Author

I'm going to mark this as draft for now, until we resolve some stuff internally. I don't want to break things with new version pins.

@opotowsky opotowsky marked this pull request as draft July 27, 2023 17:44
@john-science
Copy link
Member

@opotowsky @ntouran Arrielle, it would be nice to allow people to check out ARMI and run it on Python 3.11.

Can we make this two PRs?

  1. Get dependencies and unit tests up to date to just get Python 3.11 "unit tests" GH working.
  2. Then get another PR to do the rest of this? (ruff and docs)

@opotowsky opotowsky marked this pull request as ready for review August 7, 2023 18:25
@opotowsky
Copy link
Member Author

@opotowsky @ntouran Arrielle, it would be nice to allow people to check out ARMI and run it on Python 3.11.

Can we make this two PRs?

  1. Get dependencies and unit tests up to date to just get Python 3.11 "unit tests" GH working.
  2. Then get another PR to do the rest of this? (ruff and docs)

Among others, I reverted the ruff version pin update. I don't know why I even updated it in the first place, since the existing version is 3.11-compatible.

setup.py Show resolved Hide resolved
@@ -1,7 +1,4 @@
numpy>=1.21,<=1.23.5
ipython>=7.34.0,<=8.12.0

# docutils 0.17 introduced a bug that prevents bullets from rendering
# see https://github.com/terrapower/armi/issues/274
docutils <0.17
Copy link
Member Author

@opotowsky opotowsky Aug 7, 2023

Choose a reason for hiding this comment

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

This version pin is incompatible with python 3.11. It could stay removed and I could instead update the requirement in requirements-docs.txt (which, oddly, it has docutils <0.18 instead)

Whatcha think @john-science?

Copy link
Member

Choose a reason for hiding this comment

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

That's just for docs, right?

Or is it used outside the docs?

Copy link
Member Author

@opotowsky opotowsky Aug 7, 2023

Choose a reason for hiding this comment

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

It's used in utils:

image

But I don't think bullet point rendering matters there ;-)

It needs to be unpinned here/in setup.py to be python 3.11 compatible.

@opotowsky opotowsky added the ci Related to continuous integration / github actions label Aug 7, 2023
Copy link
Member

@john-science john-science left a comment

Choose a reason for hiding this comment

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

The only question I have is the MPI version change.

Should we run some in-house integration tests to make sure that change doesn't break anything?

Other than that, great!

@opotowsky
Copy link
Member Author

The only question I have is the MPI version change.

Should we run some in-house integration tests to make sure that change doesn't break anything?

Other than that, great!

Actually we have mpi4py version pinned in-house, so removing the version pin here won't change anything.

@opotowsky opotowsky merged commit 4654b96 into main Aug 8, 2023
@opotowsky opotowsky deleted the py311 branch August 8, 2023 00:30
@opotowsky
Copy link
Member Author

@john-science @ntouran we have py311 success!

drewj-usnctech added a commit to drewj-usnctech/armi that referenced this pull request Aug 24, 2023
…id-refactor-pre-1278

* terrapower/main:
  Move max assembly number out of global scope (terrapower#1383)
  Ensuring we throw an error on bad YAML files (terrapower#1358)
  Removing unused settings (terrapower#1393)
  Fixing some miscellaneous TODOs (terrapower#1392)
  Removing global plugin flags (terrapower#1388)
  Update reactivity coefficient parameters (terrapower#1355)
  Fixing ruff formatting of doc gallery examples (terrapower#1389)
  Fixing broken link (terrapower#1390)
  Removing unreachable code block (terrapower#1391)
  Removing unnecessary f-strings (terrapower#1387)
  Updating documentation dependencies for Python 3.11 (terrapower#1378)
  Adding a little code coverage to the CLIs (terrapower#1371)
  Remove coveragepy-lcov dependency during CI. (terrapower#1382)
  Reconnect logger after disconnecting in test. (terrapower#1381)
  Add Python 3.11 to GH Actions (terrapower#1341)
  Removing global variable from runLog.py (terrapower#1370)
  Moving the First-Time Contributors guide to the docs (terrapower#1368)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to continuous integration / github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test support for Python 3.11
2 participants