Proposal: updated dependency handling

[romanc]

Description

Recent CI failures exposed a couple of inconsistencies in how pace is handling dependencies. This PR proposes a bit of a cleanup that is a compromise between keeping the current developer workflow while "moving in the right direction" of more modern dependency handling. Eventually, we'll need to move away from setup.py towards the emerging pyproject.toml standard.

The idea of this PR is to keep the dev setup a simple default with local NDSL, pyFV3, and pySHiELD installs. While the dev setup installs an editable version of pace (but none of the submodules anymore), the CI and build_scripts/ don't have any editable installs anymore. We refrain from automatically updating pip (and setuptools/wheels) to having to deal with pip > 25, which enforces PEP600.

Bigger changes include:

• Added extras for test (installs "mpi4py", "nbmake", and "pytest") and dev (installs local but non-editable versions of NDSL, pyFV3, pySHiELD with a local version of pace and linting tools)
• Updated constraints.txt to current versions.
• Removed requirements_lint.txt and requirements_dev.txt (they are now an extra in setup.py)

Minor changes include:

• Updated README.md accordingly
• Added optional folder argument to generate_eta_files.py
• Rewrote most of the linting workflow

The docs requirements are still a bit out there and imo don't really make senses. I was unsure what to do with them. I think they should be separate from tests and the development environment. They should probably end up in their own extra such that a docs workflow could pip install .[prod,docs]. Since there is no docs workflow, I didn't bother too much.

Note that this also doesn't run tests with multiple python version. It would be relatively straight forward to run for example unit tests in all supported (3.8 - 3.11) versions.

How Has This Been Tested?

Local testing with two virtual environments; one for the developer workflow and one for the test workflow.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules: N/A
• New check tests, if applicable, are included: N/A

[romanc]

some inline comments

[romanc]

versions below 3.8 aren't supported anymore. I tried adding newer versions of python and this failed, which is probably due to the outdated version configured in .pre-commit-config.yaml.

I propose to update black in a subsequent PR and revisit this since updating black sometimes brings changes in default formatting, which would convolute this PR.

[romanc]

It's not usually common to have an upper limit. Nevertheless, we know from NDSL that python 3.12 will break things, so let's be cautious.

[romanc]

just added missing supported versions.

[romanc]

Removed the editable installs of NDSL, pyFV3, and pySHiELD from the dev setup as discussed with @fmalatino.

Let's be sure to merge this as "squash merge" because the individual commits aren't meaningful.

[fmalatino]

Aside from the local references to packages in the constraints files this is good to go. Thanks again so much for taking this on.

[fmalatino]

These types of constraints need to be removed after every update to the constraint files.

[FlorianDeconinck]

Can we do away with the constraints.txt ? I understand the idea behind it, but it seems to me that the hard limit on packages should firmly be defined in setup.py/pyproject. It does weaken releases, except if we move to a lock system.

[fmalatino]

Can we do away with the constraints.txt ? I understand the idea behind it, but it seems to me that the hard limit on packages should firmly be defined in setup.py/pyproject. It does weaken releases, except if we move to a lock system.

I am good with this, would this then require that a PR be made in pace to update the version of an incoming package alongside one in a submodule that changes it?

[fmalatino]

Approved pending the change to moving away from constraints files, and keeping versions of dependencies within setup.py. Thanks again for all of this.

[romanc]

Approved pending the change to moving away from constraints files, and keeping versions of dependencies within setup.py. Thanks again for all of this.

I will remove the constraints files again and push another version.

[romanc]

Alright, I pushed "one last" (haha) update to the README. Feel free to take another look, I might have missed something in the back and forth.

Also, there are probably some commands in the Makefile that now fail. Not sure if they are in use by anyone. Similarly, there's a circleci and a jenkins configuration that also look outdated. These would need to be updated (either here or in a follow-up PR) in case they are in still in use.

Lastly, I can also not give any guarantees for the Dockerfile and/or running pace in docker. I didn't test that part - just so you know before hitting the merge (please do squash merge) button.

[romanc]

Can we do away with the constraints.txt ? I understand the idea behind it, but it seems to me that the hard limit on packages should firmly be defined in setup.py/pyproject. It does weaken releases, except if we move to a lock system.

I am good with this, would this then require that a PR be made in pace to update the version of an incoming package alongside one in a submodule that changes it?

Let me try to answer this. Are you asking what happens if a dependency, say PyFV3, updates one of its dependencies that is in pace's list of dependencies?

In the above case, pip will try to find a version that fots both requirements. Pace's requirements are now pretty loose (except for important things like numpy < 2 and zarr < 3). So chances of resolution are high. And in case of resolution, no action is needed on the pace side. In case there's a conflict, pip will fail, which makes the workflow fail, which lets downstream repos know (in case they have the hook installed). And we will also know when attempting to update one of pace's submodule, because - again - pipeline failures.

We could setup a daily/weekly CI to run with the latest available dependencies to catch things like zarr moving to version 3 in an automated manner.

[FlorianDeconinck]

I was more referencing the fact that if you don't freeze all packages pull (dependencies of dependencies of dependencies...) then when you release a software their is no way to make sure you grab exactly those second hand dependencies later when you re-install