-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
CI: improve test suite runtime via pytest parallelism and disabling mypy in most jobs #24291
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
Conversation
Considering we have a dedicated static typing label, would it be an idea to automatically enable to env var if it's set? Situations wherein the label is present but one is not interested in running the mypy are, realistically speaking, non existent. |
That seems like a good idea to me. In case that is hard to do on, e.g., Azure (since I don't think we have any label-based logic there) we can perhaps also recognize |
The Intel SPR failures seem unrelated:
The macOS arm64 wheel build jobs looks much better: ![]() So this almost halved the runtime compared to before (see here). The Run cibuildwheel step duration decreased by more than 2x, and average CPU utilization increased from ~1 to ~2.5 (out of 4 cores) during that whole phase. |
Hm, a shame that it cannot be done automatically for Azure, but so be it; directly using the commit message is a decent plan B. Just brain storming here at this point, but how feasible would it be to automatically run it if a change is detected in one of the .pyi files? |
On GitHub Actions one can provide a list of paths to trigger on if those are touched in the PR diff. It'd be a pretty long list, but it's doable. I think on Azure it's again more tricky. Let me ask a question in return though: what kind of coverage actually makes sense for these tests? I imagine that we'd want macOS/Linux/Windows and also all covered Python versions (currently 3.9-3.12). Does it have to be a dense matrix though across all CI providers? It's hard to see why that would matter. So maybe what would be easier is to ensure we cover that set of OSes and Python versions on GitHub Actions? Or maybe we can use |
You make a good point here: all that's really important is to check the basic mac/windows/unix + Python version combinations. But anything else? Tests with specific compiler options, 32 bit windows, Etc.? They're all, at minimum, fairly irrelevant here for the purpose of typing related tests and disabling them in full on those platforms is perfectly fine IMO. As for switching to Pyright? This will be a non-trivial amount of work as our test suite is currently heavily specialised for mypy. There is #21505 on the wish list that should make adapting the test suit for different type checkers a bit easier, but again: that's just on the wish list for now. |
Thanks for confirming. I'm considering putting that in a separate GHA yaml file - then it's also triggerable by hand via "workflow dispatch" for any maintainer, and we can keep all the paths to |
GH also supports wild cars containing file paths for its trigger list, so it's probably easier to just use |
I'm marking this as draft until the new Mypy-specific CI jobs have been added. I am backporting the changes here to the |
[skip azp] [skip circle] [skip travis]
…is set These tests are super slow, and they're effectively always passing in CI. Running them on all "full" test suite runs is too expensive. Note that SciPy has an XSLOW mark, NumPy does not. So use an env var for now. [skip circle] [skip travis] [skip azp]
bd1bda6
to
3411697
Compare
[skip cirrus] [skip travis] [skip circle] [skip azp]
3411697
to
7de0dbb
Compare
This should be good to go now:
|
python -c "import sys; import numpy; sys.exit(not numpy.test(label='full'))" | ||
# Run full tests with -n=auto. This makes pytest-xdist distribute tests across | ||
# the available N CPU cores: 2 by default for Linux instances and 4 for macOS arm64 | ||
python -c "import sys; import numpy; sys.exit(not numpy.test(label='full', extra_argv=['-n=auto']))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it is worthwhile distributing tests when there are only 2 cores? Does the total time go down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's faster. typically ~50% or so
This comment mentions triggering on the "Static Typing" label and on a |
It's run by default already, so that's not needed. We shouldn't turn it off completely for regular Python/C work in the package, because that can break static typing |
Thanks @rgommers |
This is a backport of numpy#24493 and numpy#24291. The purpose of this is to ease future backports that expect these files. - CI: move some jobs in `build_test.yml` to Meson - CI: split `build_test.yml` into three GHA jobs files Also documents better what is being run. See numpygh-24410 for the overall restructuring plan for GitHub Actions CI. - CI: merge `linux_meson.yml` into `linux_blas.yml` - TST: disable mypy tests in test suite unless an environment variable is set These tests are super slow, and they're effectively always passing in CI. Running them on all "full" test suite runs is too expensive. Note that SciPy has an XSLOW mark, NumPy does not. So use an env var for now. - CI: add new GHA CI jobs to run MyPy across OS/Python flavors
This is a backport of numpy#24493 and numpy#24291. The purpose of this is to ease future backports that expect these files. - CI: move some jobs in `build_test.yml` to Meson - CI: split `build_test.yml` into three GHA jobs files Also documents better what is being run. See numpygh-24410 for the overall restructuring plan for GitHub Actions CI. - CI: merge `linux_meson.yml` into `linux_blas.yml` - TST: disable mypy tests in test suite unless an environment variable is set These tests are super slow, and they're effectively always passing in CI. Running them on all "full" test suite runs is too expensive. Note that SciPy has an XSLOW mark, NumPy does not. So use an env var for now. - CI: add new GHA CI jobs to run MyPy across OS/Python flavors
Should address some of the issues identified in gh-24280.
Regarding the Mypy tests: these tests are super slow, and they're effectively always passing in CI. Running them on all "full" test suite runs is too expensive, so enable them on a single job instead. Note that SciPy has an XSLOW mark, NumPy does not. So use an env var for now. Typical guidelines are that fast tests should be >0.5 sec, slow only in the 0.5-5 sec range, and anything above that should be avoided. The boundaries are fuzzy, but Mypy takes 1-2 minutes to run, so needs custom opt-in. Cc @BvB93 for visibility.
Also adds a
spin mypy
developer command to make it easier to run only static typing tests.