Skip to content
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

Modernise packaging and CI #294

Merged
merged 11 commits into from
Jul 27, 2023
Merged

Modernise packaging and CI #294

merged 11 commits into from
Jul 27, 2023

Conversation

ZedThree
Copy link
Member

Fixes #253

  • Switch completely to pyproject.toml, remove setup.cfg, setup.py
  • Various updates to CI, including:

Minimum supported version for python 3.8
- install dependencies from package
- install package in separate step
- Delete tests from publish workflow, these should already have run!
- Use credential-less "trusted publisher" workflow, see: https://docs.pypi.org/trusted-publishers/
@ZedThree
Copy link
Member Author

pytest-min-versions is basically try to use incompatible versions:

ERROR: Ignored the following versions that require a different python version: 1.25.0 Requires-Python >=3.9; 1.25.0rc1 Requires-Python >=3.9; 1.25.1 Requires-Python >=3.9; 2023.2.0 Requires-Python >=3.9; 2023.3.0 Requires-Python >=3.9; 2023.4.0 Requires-Python >=3.9; 2023.4.1 Requires-Python >=3.9; 2023.4.2 Requires-Python >=3.9; 2023.5.0 Requires-Python >=3.9; 2023.5.1 Requires-Python >=3.9; 2023.6.0 Requires-Python >=3.9; 2023.6.1 Requires-Python >=3.9; 2023.7.0 Requires-Python >=3.9; 2023.7.1 Requires-Python >=3.9
ERROR: Could not find a version that satisfies the requirement animatplot-ng==0.4.2 (from versions: 0.4.3)
ERROR: No matching distribution found for animatplot-ng==0.4.2

Guessing the first line is due to xarray?

Is it still a useful test overall?

@dschwoerer
Copy link
Contributor

ERROR: Could not find a version that satisfies the requirement animatplot-ng==0.4.2 (from versions: 0.4.3)
That was my mistake - I shouldn't have switched them to animatplot-ng

I am happy to remove those tests.
I am confused how xarray manages to prevent installing on python < 3.9 - I would assume the old versions did not include that at some point? Are they pulling in something, that requires a new python?

.github/workflows/master.yml Outdated Show resolved Hide resolved
.github/workflows/master.yml Outdated Show resolved Hide resolved
.github/workflows/pythonpackage.yml Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
Also explicitly use py 3.11 in tests

Co-authored-by: David Bold <dschwoerer@users.noreply.github.com>
@ZedThree
Copy link
Member Author

I think maybe an issue with the version of netcdf4?

Collecting netcdf4==1.4.2
  Downloading netCDF4-1.4.2.tar.gz (769 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 769.8/769.8 kB 53.3 MB/s eta 0:00:00
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
  Installing backend dependencies: started
  Installing backend dependencies: finished with status 'done'
  Preparing metadata (pyproject.toml): started
  Preparing metadata (pyproject.toml): finished with status 'error'
  error: subprocess-exited-with-error
  
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [18 lines of output]
      <string>:170: DeprecationWarning: The SafeConfigParser class has been renamed to ConfigParser in Python 3.2. This alias will be removed in future versions. Use ConfigParser directly instead.
      reading from setup.cfg...
      using nc-config ...
      Traceback (most recent call last):
        File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 149, in prepare_metadata_for_build_wheel
          return hook(metadata_directory, config_settings)
        File "/tmp/pip-build-env-hmw2rbma/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 380, in prepare_metadata_for_build_wheel
          self.run_setup()
        File "/tmp/pip-build-env-hmw2rbma/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 487, in run_setup
          super(_BuildMetaLegacyBackend,
        File "/tmp/pip-build-env-hmw2rbma/overlay/lib/python3.8/site-packages/setuptools/build_meta.py", line 338, in run_setup
          exec(code, locals())
        File "<string>", line 457, in <module>
      ModuleNotFoundError: No module named 'numpy'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

@johnomotani
Copy link
Collaborator

Is pytest-min-versions still a useful test overall?

I think it's useful in principle, but I'd be happy to bump the versions up a bit. I think it's mostly been useful for when xarray introduces some new functionality/syntax - if you change something that uses a new feature it might break xBOUT for users with an older xarray. Even then it's fine to force them to upgrade, but you need to know to bump the minimum required version required for xarray, etc.

This version of netcdf4 doesn't have a binary wheel for py3.8, so gets
built from source and needs numpy at build-time. Later versions do
specify numpy as a built requirement.
@ZedThree
Copy link
Member Author

Ok, having looked into this a bit more, the min-version test is nice in principle, but very fragile -- figuring out a consistent set of minimal versions that will work is very tricky.

I'm struggling to install a set of minimal dependencies even locally.

It might be best to drop this test completely. xarray is already pinned to >=0.18.0,<2022.9.0, so that would hopefully catch any issues with using newer features?

@johnomotani
Copy link
Collaborator

I think everything except xarray has a pretty stable API, and is not that likely to be called directly anyway from xBOUT. I'd be happy to do a 'min versions test' that just installs xarray-0.18.0, and lets everything else have whatever version it likes. Does that seems easier to get working?

The 'min versions' test has saved me a few times from using something that was only available in the very latest xarray, so it would be nice to keep in some form if possible.

@ZedThree
Copy link
Member Author

Sure, happy to do that. Just to be clear, the other tests will only install xarray 2022.9.0, so they will also catch features only in more recent versions too.

@ZedThree ZedThree merged commit 216c14b into master Jul 27, 2023
11 checks passed
@ZedThree ZedThree deleted the modernise-packaging branch July 27, 2023 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Update actions
3 participants