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

[CI] Just test import for wheels #453

Merged

Conversation

psobolewskiPhD
Copy link
Contributor

Currently, PR workflows run tests and build wheels.
Meanwhile, the wheel workflow runs full tests again.
This PR changes the wheel tests to just check that numcodecs is import-able, to reduce CI time.
See #427 (comment) and followup thread.

Note: could add something more to the command beyond importing, but I'm not sure what would be useful.

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

@psobolewskiPhD
Copy link
Contributor Author

Just a note the concurrency worked perfectly!

@psobolewskiPhD
Copy link
Contributor Author

psobolewskiPhD commented Jul 30, 2023

Wheels look like ~15 min total now:
https://github.com/zarr-developers/numcodecs/actions/runs/5702970684?pr=453
vs. 20+ on this docs update PR:
https://github.com/zarr-developers/numcodecs/actions/runs/5702663996?pr=426

(note: build steps are slower here, so maybe different runners)

@psobolewskiPhD
Copy link
Contributor Author

The original build with arm64 macOS PR:
image
This PR:
image
Older, unrelated docs PR (no changes to wheels, no arm64 macOS)
image

Not sure the runners are reliably the same, but the testing time savings is greatest on macOS.
Python 3.8 is particularly slow BTW (and no longer supported by numpy...)

@joshmoore
Copy link
Member

Considering msgpack, etc. were not installed by the wheels build anyway, the call to pytest couldn't currently have been comprehensive. I imagine there's more unification that could happen between the builds, but for the moment, this seems like a straight-forward enough simplification. 👍

@joshmoore joshmoore merged commit 7cbf40d into zarr-developers:main Aug 3, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants