-
Notifications
You must be signed in to change notification settings - Fork 442
chore: #3847 Update min python version to 3.11 #3901
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
|
Need to upgrade pyproj version. It's being built from source in the pipeline due to the Python version change (or setup so that builds from source work on older pyproj). |
|
You are absolutely not overstepping, thanks for opening this! We're aiming to cut a bug fix release in the coming weeks, and I'll confirm with @dopplershift about saving this for after or not. Also, looks like SciPy >= v1.9.2 (2022) gives us Python 3.11 wheels if we don't want to deal with these build failures on minimum. Otherwise we gotta plumb in libopenblas I think. I doubt you've missed anything here, but I'll take a proper look later this week. In the meantime, glad to shout out a few of your points!
We've avoided this, or taking on a formatter, up to this point, and I think has historically benefitted our community of scientists/researchers/students, often first time PR authors. Instead we opt for the "document, describe, offer feedback" approach to avoid any additional dev setup and potential barriers to contributing. It does make the review and check process a bit more burdensome once a PR is open though. We still go back and forth on this pretty regularly.
I've been messing around with uv and pixi on other projects this Summer, and have enjoyed the experience. We're currently migrating our build system to (likely) scikit-build-core + cibuildwheel to support upcoming compiled extensions, eg #3856. I doubt we'll change any of the current packaging/build/CI setup until after, or during only if necessary/digestibly doable, those changes come in. I don't oppose the discussion though!
Only doable if/when we stop relying on flake8 for specific community plugins and/or our own small plugin for internal unit quantity constructors. The former has come up recently in discussion. The latter hasn't been discussed in some time I don't think.
I haven't looked into anything like this, but am interested and curious! |
|
@dcamron Thanks for taking a look and for the thorough reply :) Trying with 1.9.2..
Didn't think about this (mostly working with long running contributors) but first time PR authors is honestly a good point. Also you have linting runs anyways on CI. Something I've seen other projects do (again take it or leave it, just sharing) is have CI run the fixes itself and commit automatically. This always makes me a bit uneasy though..
Completely fair, this is a complex project I know almost literally nothing about :S
We've had a good experience with pytest-xdist, but if you have compute heavy tests (I would imagine you do) it could be more harm than good. Might be worth playing around with before committing one way or the other. I might play around in the background to see what I find. |
|
BTW not sure what you want to do with the failing code coverage. It's failing due to |
|
matplotlib/pytest-mpl#206 (comment) looks like you know about this issue? |
dopplershift
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.
Thanks for contributing the cleanup! Just one minor issue where the wrong side of the if got kept.
Let me know if you're not in a position to update and I can push the changes and merge.
|
Hey @dopplershift, thanks for the review and sorry for the drive by PR! I can for sure make these changes in the next few days |
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.
Pull Request Overview
This PR updates the minimum supported Python version from 3.10 to 3.11, aligning the codebase with Python 3.11+ features and updating dependencies accordingly.
Key Changes:
- Replaced
timezone.utcwith the Python 3.11+UTCconstant across all datetime usage - Removed version-specific conditional code for Python 3.11 features (e.g., 'z' format specifier)
- Updated minimum dependency versions for
pint,pyproj, andscipyto versions compatible with Python 3.11
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/metpy/remote/aws.py | Updated all timezone.utc references to UTC constant |
| src/metpy/plots/station_plot.py | Removed Python version check and uses 'z' format specifier directly |
| src/metpy/plots/_util.py | Updated timezone.utc to UTC in timestamp function |
| src/metpy/io/text.py | Updated timezone.utc to UTC in WPC bulletin parser |
| src/metpy/io/nexrad.py | Updated timezone.utc to UTC in NEXRAD datetime conversion |
| src/metpy/io/metar.py | Updated timezone.utc to UTC in METAR parsing |
| pyproject.toml | Updated requires-python to 3.11 and removed 3.10 classifier; bumped minimum versions for pint, pyproj, and scipy |
| examples/meteogram_metpy.py | Updated to use dt.UTC instead of dt.timezone.utc |
| docs/userguide/installguide.rst | Updated documentation to reflect Python >= 3.11 requirement |
| docs/index.rst | Updated documentation to reflect Python >= 3.11 requirement |
| README.md | Updated README to reflect Python >= 3.11 requirement |
| CONTRIBUTING.md | Updated testing documentation and added mention of uv as installation option |
| .python-version | Removed file that specified Python 3.10 |
| .github/workflows/tests-pypi.yml | Updated test matrix to use Python 3.11 as minimum version |
| .github/workflows/tests-conda.yml | Updated test matrix to use Python 3.11 as minimum version |
| .github/workflows/docs.yml | Updated documentation build matrix to remove Python 3.10 |
| .github/workflows/docs-conda.yml | Updated documentation build matrix to use Python 3.11 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
77d732e to
5b50126
Compare
|
As we wind this down, I realize I never responded to some good suggestions above. Some comments:
Some interest, but last I looked there were problems pinning versions, specifically for pinning flake8 plugins. This may be less of an issue these days as we have moved most things to ruff.
@dcamron has been looking at pixi in #3919. As we embark on having compiled code in MetPy, there's definitely some changes coming to the CI pipeline, and we can consider any uv/pixi support with that. Real question is testing PyPI vs. conda-forge ecosystems.
Would be nice, but so long as we're relying on flake8, that's a non-starter. I think I've seen plugins to address this, but I'm not sure depending on yet more other code is better than having a harmless config file around.
That'd be awesome, but right now it doesn't accept how our imports are formatted (we need isort's default "grid"), and I hate how ruff does it.
Interested. Happy to discuss in #3906 (which I need to get over on) |
- pyproj 3.6.1 added py 3.12 wheels: https://github.com/pyproj4/pyproj/releases/tag/3.6.1
broken import from pint causing broken build: hgrecco/pint#1618
Not needed since we dropped support for these older versions.
5b50126 to
5eb9e9d
Compare
Description Of Changes
Closes #3847
Not relevant to this MR, but may be nice to have in future (and I would be happy to add if there's interest):
uv? This may be controversial - happy to drop it. Reasons to do so would be a) faster install time b) locked dependencies (easier to manage and don't break randomly) c) easier to upgrade dependencies (uv lock --upgrade). Reasons not to a) Some devs may prefer/need conda b) I'm not familiar enough with the package to see other flaws.setup.cfg, move to usingpyproject.tomlfully.ruff(I know this may not be fully possible now due toflake8-continuationwarnings & maintenance #3866)Hope this is not overstepping, feel free to discard this PR. Have a nice day!
Checklist