Update supported python versions

altheaden commented 2 months ago

Support for Python 3.8 (and lower) has been dropped from conda-forge, so I've updated the lower bounds of supported versions here accordingly.

Updated template from @forsyth2

Issue resolution

Equivalent to https://github.com/E3SM-Project/e3sm_diags/pull/844, https://github.com/E3SM-Project/zstash/pull/351

This pull request is a minor adjustment that would increment the patch version.

1. Does this do what we want it to do?

Objectives:

Make Python 3.9 the oldest supported Python version in zppy.

Required:

[x] Product Management: I have confirmed with the stakeholders that the objectives above are correct and complete.
[ ] Testing: I have added or modified at least one "min-case" configuration file to test this change. Every objective above is represented in at least one cfg.
- Existing tests are run in an environment already using Python 3.9.
[x] Testing: I have considered likely and/or severe edge cases and have included them in testing.

2. Are the implementation details accurate & efficient?

Required:

[x] Logic: I have visually inspected the entire pull request myself.

3. Is this well documented?

Required:

[ ] Documentation: by looking at the docs, a new user could easily understand the functionality introduced by this pull request.
- This change does not require new documentation.

4. Is this code clean?

Required:

[x] Readability: The code is as simple as possible and well-commented, such that a new team member could understand what's happening.
[x] Pre-commit checks: All the pre-commits checks have passed.

xylar commented 2 months ago

@forsyth2, could you allow CI to run on this PR and #621? What kind of testing would you like to see for these 2 PRs? We're mostly just trying to apply the same clean up here (and on zstash) that we're applying to other E3SM-related python packages.

forsyth2 commented 1 month ago

@altheaden My original plan was to run the integration tests on this PR after fixing #625 (which #628 I merged this morning does). But it now occurs to me that wouldn't actually show anything new since my dev environment uses Python 9 anyway:

$ python --version
Python 3.9.13

However, I notice the unit test step (CI/CD Build Workflow > Details > Run Unit Tests) shows only 2 unit tests rather than the new number of 22 (post-merging of #628). I thought the CI/CD rebased on the latest main but clearly it does not. In that case, I would say if you can rebase this PR on the latest main and make sure the 22 unit tests pass, this should be good to merge.

I think the same would go for #621 and #627. I do notice #627 has some merge conflicts. (Sorry, I did some pretty extensive refactoring in #628 to make zppy's code easier to understand and to test.)

altheaden commented 1 month ago

@forsyth2 Just did a rebase onto the latest main, can you re-run CI when you get a chance?

forsyth2 commented 1 month ago

Oh I just noticed that zppy/setup.py still has

python_requires=">=3.6",

Should that be changed too, as in https://github.com/E3SM-Project/zstash/pull/351?

altheaden commented 1 month ago

@forsyth2 I must have missed that here, I'm sorry - let me create another PR to fix that. Thanks for catching that.

forsyth2 commented 1 month ago

No worries, thanks @altheaden!

E3SM-Project / zppy