Update to packaging tools and logic

[dougiesquire]

This PR implements some changes discussed in #398:

• Moved to pyproject.toml. Removed setup.cfg
  • install optional test and mitgcm dependencies now with e.g. pip install .[test]
  • it seems that the test and doc dependencies are bundled together. I’ve left this for now, but in the future it might be tidier to split them
• Added versioneer support
• Update conda build to build from PyPI dist. I've modified the CD Github workflow so that the conda build only happens if the PyPI upload is successful. NO LONGER TRUE
• Tidied up the CD Github workflow, functionality unchanged
• Added a pypa-build job to the CI Github workflow and renamed the old build job to tests

In doing this, I discovered that the um driver requires imp, which has been deprecated. I’ve updated the driver to use the replacement importlib instead and the tests still pass, but I'm not sure the tests actually touch the modified code...

I've also removed the tools and bin directories (#396 and #397). This doesn't have to be done here, but it felt relevant since these are places where the hard-coded version was being used.

I couldn’t work out a easy/tidy way to have dependencies defined in only one location for the PyPI and conda builds, so dependencies are currently listed in both pyproject.toml and meta.yaml. NO LONGER TRUE - the dependencies and entry points in meta.yaml are input dynamically from pyproject.toml.

I’ve tested building the PyPI dist, uploading to TestPyPI and then doing the conda build locally from that. But the true test will be actually issuing a new release.

Closes #398, closes #401, closes #397, closes #396

[pep8speaks]

Hello @dougiesquire! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file payu/_version.py:

Line 381:80: E501 line too long (84 > 79 characters)
Line 471:80: E501 line too long (80 > 79 characters)
Line 474:80: E501 line too long (84 > 79 characters)

Comment last updated at 2024-01-22 04:26:34 UTC

[dougiesquire]

The conda-build job that's been added to CI.yaml will need to be altered or removed. Is it necessary anymore?

[coveralls]

coverage: 51.69% (-0.9%) from 52.591%
when pulling e094ac4 on dougiesquire:issue-398
into ca0c825 on payu-org:master.

[CodeGat]

Hey @dougiesquire! With regards to #403 (comment)
I think we'd need the equivalent check for PyPI, in this case. We used the conda build check to make sure we weren't putting anything broken onto vk83 (which had happened before). Is there a similar thing where you can attempt to build Payu using PyPI?

[dougiesquire]

I think we'd need the equivalent check for PyPI, in this case. We used the conda build check to make sure we weren't putting anything broken onto vk83 (which had happened before). Is there a similar thing where you can attempt to build Payu using PyPI?

Sure. The current build job in the CI is really more about running the tests. So we could rename this to test and add a build job to build the distribution with PyPA's build package? Of course, success of that job still won't guarantee a successful conda build.

We could do something like upload the dist resulting from the PyPA build to TestPyPI and then try and build the conda build from that, but I'm not sure how the tag-based versioning would work...

[dougiesquire]

I've worked out a way to dynamically input the dependencies in meta.yaml from the pyproject.toml. However, it requires that the dependencies are listed in the pyproject.toml using the slightly more restricted syntax expected by conda (i.e. a space before version limits, but not after, e.g. package >=1.0). It also obviously will fail if the conda dependencies differ from the pip dependencies. @aidanheerdegen do we want to do this? I can add a comment to the pyproject.toml about the syntax. ADDED: see this commit.

Regarding the build tests in the CI, I see three options:

1. only build with PyPA's build package. Passing this isn't necessarily an indication that a release will be successful as the upload to PyPI or the conda build from PyPI could have untested issues.
2. Build with PyPA's build package and conda build from local source. These can be independent jobs. This will require some jinja templating in the meta.yaml to enable the source to be switched dynamically (e.g. based on an env variable set in the Github workflow) since we will want to build from local source in CI and PyPI in CD.
3. Build with PyPA's build package, upload to TestPyPI and conda build from TestPyPI dist. These will be dependent jobs. Again jinja templating will be required to switch the PyPI url depending on whether CI (https://test.pypi.org) or CD (https://pypi.io).

Curious on thoughts @CodeGat, @aidanheerdegen. 1 or 3 are probably my most preferred options. ADDED: e.g. 3 might look something like this commit.

[dougiesquire]

I can't work out why the trusted publishing to TestPyPI is not working (it says here that it's supported). I'm going to stop working on this and give @aidanheerdegen and @CodeGat some time to respond to my questions above. And if anyone has ideas about why trusted publishing isn't working, please let me know!

[aidanheerdegen]

I've worked out a way to dynamically input the dependencies in meta.yaml from the pyproject.toml. However, it requires that the dependencies are listed in the pyproject.toml using the slightly more restricted syntax expected by conda (i.e. a space before version limits, but not after, e.g. package >=1.0).

This looks like a great approach. Jinja templating is a standard feature of conda/meta.yaml so it isn't going to mysteriously stop working. The requirement to match the stricter syntax for conda version pinning is a feature IMO. We already had an issue with this when duplicating requirements from setup.py to meta.yaml. Being explicit and doing it one place is much better. Just need to make sure we test for correctness in the CI (by test deployment, or linting if that is an available option).

Regarding the build tests in the CI, I see three options

One clear goal is that the PyPI and conda distributions are the same/consistent/identical, and that there is minimal overhead in maintaining that.

One route to making the deployment identical is to deploy to PyPI and then deploy conda from PyPI which is what condo-forge does.

We’re running our CD for PyPI and conda from the same repo so we can orchestrate it to build the package locally and deploy to PyPI and then build conda from the same local version of the built package directly, rather than downloading it from PyPI. Right? That corresponds roughly to your option 2 I believe?

I can't see a lot of downside to that.

[dougiesquire]

One route to making the deployment identical is to deploy to PyPI and then deploy conda from PyPI which is what condo-forge does.

This is what I've currently set up in CD.yml and it should work for releases. But we wanted to also add test builds to CI.yml to catch issues early. Obviously we don't want to upload to PyPI as part of this (because that would be a release), so I've tried to upload to TestPyPI. Unfortunately the trusted publishing is failing when trying to upload to TestPyPI. I don't understand why - others are doing this successfully, e.g. xarray.

I can't see a lot of downside to that.

Yeah, I don't think there's a downside. I liked the idea of also testing the upload using TestPyPI and got fixated on why it's not working. I'll try creating a new PyPI account and if that doesn't fix the issue I'll give up.

[dougiesquire]

Okay, I gave up on trying to test the upload via TestPyPI. The conda build is now being done from local source, rather than from the PyPI build. So not that much has actually changed in the end. I'll edit the original description to be accurate about what's changed and then I think I'll leave it there.

Sorry for the messy commit history. I took the "suck it and see" approach to debugging ci workflows...

[aidanheerdegen]

Looks great. Thanks for a comprehensive piece of work that ended up touching a lot.

[jo-basevi]

It all looks good to me. I didn't know much about PyPI builds, versioneer and jinja templating so learnt a bit - Thanks!

[dougiesquire]

Ugh sorry, did my OCD changing of the title dismiss your reviews @aidanheerdegen and @jo-basevi?

[aidanheerdegen]

Looks like it eh?

[jo-basevi]

LGTM 👍

[aidanheerdegen]

LGTM (still)