-
Notifications
You must be signed in to change notification settings - Fork 14
Update Python support #735
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
Conversation
Testinglcrc_conda
cd ~/ez/zppy
git fetch upstream main
git status
# Check for uncommitted changes
git checkout -b update-python upstream/main
git grep -n python
# Make changes to support python3.11+
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-py-3-11-20250916
conda activate zppy-py-3-11-20250916
pre-commit run --all-files
python -m pip install .
pytest tests/test_*.py
# 25 passed
git add -A
git commit -m "Update Python support" |
.github/workflows/build_workflow.yml
Outdated
| path: ~/conda_pkgs_dir | ||
| key: ${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{ | ||
| hashFiles('conda/dev.yml') }} | ||
| hashFiles('conda/dev.yml') }}}-python${{ matrix.python-version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove 3rd }
|
@forsyth2 , please add me as a reviewer when this is ready for review. |
.github/workflows/build_workflow.yml
Outdated
| channel-priority: strict | ||
| channel-priority: flexible # Changed from strict to flexible | ||
| auto-update-conda: true | ||
| python-version: "3.11" # Use stable Python version for docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to 3.13
| - docutils>=0.16 # Removed <0.17 constraint | ||
| # Quality Assurance Tools | ||
| # ======================= | ||
| # If versions are updated, also update 'rev' in `.pre-commit-config.yaml` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do this: "If versions are updated, also update 'rev' in .pre-commit-config.yaml"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The autoupdate didn't seem to work for zppy-interfaces: E3SM-Project/zppy-interfaces#33 (comment), so I didn't do it here either.
|
@tomvothecoder Thanks for reviewing E3SM-Project/zstash#384. Can you also take a look at this PR for @xylar also tagging you for review, since you requested. Thanks! |
conda/dev.yml
Outdated
| - black >=24.0.0 | ||
| - flake8 >=7.0.0 | ||
| # This line also implicitly installs isort | ||
| - flake8-isort=6.1.1 # version from https://anaconda.org/conda-forge/flake8-isort | ||
| - mypy=1.11.2 # version from https://anaconda.org/conda-forge/mypy | ||
| - pre-commit=4.0.1 # version from https://anaconda.org/conda-forge/pre-commit | ||
| - tbump=6.9.0 | ||
| - flake8-isort >=6.0.0 | ||
| - mypy >=1.11.0 | ||
| - pre-commit >=4.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same suggestion here. Use the latest versions and pin them with ==, just like in zppy-interfaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest that you update .pre-commit-config.yaml as well in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in c108327. Looks like it's passing 3.11, 3.12, 3.13.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A note for the near future: consider moving to a pyproject.toml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take this on once this PR goes in. It's pretty trivial with copilot.
|
One more place to update: I don't know if this should be python 3.11 (the lowest supported version) or python 3.13 (the highest). What is being done elsewhere, @tomvothecoder? |
conda/dev.yml
Outdated
| - tbump >=6.9.0 | ||
| # Developer Tools | ||
| # ======================= | ||
| - tbump=6.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is listed twice:
| - tbump=6.9.0 |
There isn't a set guideline on what this should be (min, max, or range). I decided to go with the min supported Python when I was using |
So does that mean change |
@xylar @tomvothecoder I addressed these comments in 4c5f285. The GitHub Actions are passing for 3.11, 3.12, 3.13. Is this good to merge? |
xylar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks right to me!
tomvothecoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Objectives:
Select one: This pull request is...
Small Change