Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
TST: Add future dependency tests as a weekly CI job#21634
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
run: | | ||
xvfb-run -a python -mpytest -raR -n auto \ | ||
--maxfail=50 --timeout=300 --durations=25 --log-level=DEBUG \ | ||
-W error::UserWarning |
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.
We actually would like to see the DeprecationWarning's as well (-W error
without the specific UserWarning), but on Python 3.10 Numpy's testing import causes deprecation warnings due to the distutils future removal in Python 3.12. So, we would have to wait for Numpy's removal of distutils before we did that. Unless I'm missing a way to catch a specific string of deprecation warnings so we could ignore only that specific warning.
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.
Sure, you can be more specific with warnings filters:https://docs.python.org/3/library/warnings.html#describing-warning-filters
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.
Thanks for pointing that out. I tried a few things out and it was more complicated that I hoped.
See here for other discussion:https://bugs.python.org/issue43862
While the Python API warnings.filterwarnings(action, message="", ...) uses the message as a regular expression, -W and PYTHONWARNINGS require to matchexactly thewhole message.
I got one match, and then another import failed, so not sure how many options/messages we want to match on if we do. I couldn't figure out how to use the "module" piece either, I thought that could refer to errors raise fromdistutils
directly, or even by puttingnumpy.testing
in there for where the warnings were arising from, but neither of those worked for me...
In summary, I'd leave it with the UserWarning -> Error for now and deal with DeprecationWarnings after Numpy fixes their distutils imports.
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.
Alternatively, you could changepytest.ini
to ignore what you want, as it seems to support regex.
cat >> pytest.ini <<EOFfilterwarnings = error:message:DeprecationWarningEOF
I think some of the 3.10 + numpy-1.22.dev failures are fixed on numpy's default branch (at one point I was seeing issuse with distutils warnings, but they are gone for me locally). I am in principle 👍🏻 👍🏻 on this idea! |
Looks like the test suite is passing with the nightly pandas and numpy wheels now. https://github.com/greglucas/matplotlib/runs/4290939395?check_suite_focus=true |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
run: | | ||
xvfb-run -a python -mpytest -raR -n auto \ | ||
--maxfail=50 --timeout=300 --durations=25 --log-level=DEBUG \ | ||
-W error::UserWarning |
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.
Alternatively, you could changepytest.ini
to ignore what you want, as it seems to support regex.
cat >> pytest.ini <<EOFfilterwarnings = error:message:DeprecationWarningEOF
Some of the tests seem to be a little flaky when they are run:https://github.com/greglucas/matplotlib/actions/runs/1495511858 |
I pushed a local copy of this up to my fork:https://github.com/greglucas/matplotlib/runs/4621007693?check_suite_focus=true |
The result images should be obtainable on the Summary page. It doesn't look like the failures are anywhere other than test images. |
Yep, I did download them and couldn't find any obvious reason for the failures (they are all quite small). The largest one |
Putting some more notes down here: I can't reproduce these failures on a local macosx or ubuntu machine either, so my debugging capability here is limited unfortunately. |
Perhaps this is related to AVX512 instructions now being available in the wheel... On the 3.8-3.10 runners:
On the 3.7 runner (numpy 1.21):
My local Ubuntu is on a chip without AVX512 instructions, which could explain why I'm not able to reproduce this locally. If someone does have a system with a chip that supports AVX512 and could test locally that could verify this guess: |
I suspect this is correct - I'm seeing similar small floating point differences on another project that seems to be related to both numpy version and the specific machine CI is being run on. |
I decided to rework this and incorporate it into the current tests.yml file, which seems better than trying to maintain two copies of the CI file. There are new floating-point issues that cropped up again in Normalize with longdouble. It must be a new numpy optimization in |
Uh oh!
There was an error while loading.Please reload this page.
d0e5605
tob4d1b14
CompareTest future numpy and pandas versions with Matplotlib to see ifanything needs to be done in the future to address deprecationsor other pending changes.Turns all warnings into errors, but filters out the distutilsdeprecation and find_spec warnings.
# This returns exactly 0.5 when longdouble is extended precision (80-bit), | ||
# but only a value close to it when it is quadruple precision (128-bit). | ||
assert 0 <norm(1 + 50 * eps) < 1 | ||
np.testing.assert_array_almost_equal_nulp(norm(1 + 50 * eps), 0.5) |
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.
This seems to be a bit too strict now, as it is failing on aarch64, ppc64le, and s390x, where the result is 0.50096339.
Also, on my 64-bit AMD system, which is apparently usingnp.float128
fornp.longdouble
(though I don't know if that means 80-bit internally), it seems to return 0.5 exactly, which seems the opposite of the comment.
PR Summary
Test future numpy versions with Matplotlib to see if anything needs to be done in the future to address deprecations
or other pending changes.
Link to a run on my branch (it should only run on matplotlib/matplotlib now)
https://github.com/greglucas/matplotlib/runs/4204821100?check_suite_focus=true
3.9 + numpy-1.22.dev failures
FAILED lib/matplotlib/tests/test_units.py::test_plot_masked_units[png] - matp...= 1 failed, 8352 passed, 148 skipped, 13 xfailed, 4 xpassedin 537.93s (0:08:57) =
3.10 + numpy-1.22.dev failures
FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[png] - matplotlib.tes...FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[svg] - matplotlib.tes...FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[pdf] - matplotlib.tes...FAILED lib/matplotlib/tests/test_backends_interactive.py::test_webagg - Asser...FAILED lib/matplotlib/tests/test_units.py::test_plot_masked_units[png] - matp...FAILED lib/matplotlib/tests/test_streamplot.py::test_direction[png] - matplot...FAILED lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py::test_axis_direction[png]FAILED lib/mpl_toolkits/tests/test_mplot3d.py::test_trisurf3d[png] - matplotl...FAILED lib/mpl_toolkits/tests/test_mplot3d.py::test_stem3d[png] - matplotlib....= 9 failed, 8344 passed, 148 skipped, 13 xfailed, 4 xpassed, 4 warningsin 414.94s (0:06:54) =
Additional things to consider
Perhaps we should also consider adding a nightly cibuildwheel to that repository for others to test out Matplotlib and report back to us easier?
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).