From d0510fd9d8d9d1c46ffc18099a5f771d4da3a093 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 4 May 2021 00:55:04 -0400 Subject: [PATCH 1/8] ci: Enable PyPy wheels on Windows. PyPy does not correctly define `PyErr_SetFromWindowsErr`: https://foss.heptapod.net/pypy/pypy/-/issues/3533 --- .github/workflows/cibuildwheel.yml | 2 +- src/_c_internal_utils.c | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index 1c4076d6bb3a..6cbec141f7d3 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -93,7 +93,7 @@ jobs: CIBW_BUILD: "pp37-*" CIBW_BEFORE_BUILD: pip install certifi numpy==${{ env.min-numpy-version }} CIBW_ARCHS: ${{ matrix.cibw_archs }} - if: runner.os != 'Windows' && matrix.cibw_archs != 'aarch64' + if: matrix.cibw_archs != 'aarch64' - name: Validate that LICENSE files are included in wheels run: | diff --git a/src/_c_internal_utils.c b/src/_c_internal_utils.c index 532824c5d05b..7209b4f396fc 100644 --- a/src/_c_internal_utils.c +++ b/src/_c_internal_utils.c @@ -68,7 +68,13 @@ mpl_GetCurrentProcessExplicitAppUserModelID(PyObject* module) wchar_t* appid = NULL; HRESULT hr = GetCurrentProcessExplicitAppUserModelID(&appid); if (FAILED(hr)) { +#if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030600 + /* Remove when we require PyPy 7.3.6 */ + PyErr_SetFromWindowsErr(hr); + return NULL; +#else return PyErr_SetFromWindowsErr(hr); +#endif } PyObject* py_appid = PyUnicode_FromWideChar(appid, -1); CoTaskMemFree(appid); @@ -89,7 +95,13 @@ mpl_SetCurrentProcessExplicitAppUserModelID(PyObject* module, PyObject* arg) HRESULT hr = SetCurrentProcessExplicitAppUserModelID(appid); PyMem_Free(appid); if (FAILED(hr)) { +#if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030600 + /* Remove when we require PyPy 7.3.6 */ + PyErr_SetFromWindowsErr(hr); + return NULL; +#else return PyErr_SetFromWindowsErr(hr); +#endif } Py_RETURN_NONE; #else From 00c63a318256a5ce83cda9baab9e21115ee45bb2 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Fri, 13 Aug 2021 21:34:42 -0400 Subject: [PATCH 2/8] pypy: Skip tests that require CPython. These tests look at reference counts of objects, which are meaningless for PyPy (and also Jython). --- lib/matplotlib/tests/test_quiver.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/matplotlib/tests/test_quiver.py b/lib/matplotlib/tests/test_quiver.py index d7a848f61bcc..ca0bf19de5e7 100644 --- a/lib/matplotlib/tests/test_quiver.py +++ b/lib/matplotlib/tests/test_quiver.py @@ -1,6 +1,9 @@ +import platform +import sys + import numpy as np import pytest -import sys + from matplotlib import pyplot as plt from matplotlib.testing.decorators import image_comparison @@ -15,6 +18,8 @@ def draw_quiver(ax, **kw): return Q +@pytest.mark.skipif(platform.python_implementation() != 'CPython', + reason='Requires CPython') def test_quiver_memory_leak(): fig, ax = plt.subplots() @@ -27,6 +32,8 @@ def test_quiver_memory_leak(): assert sys.getrefcount(ttX) == 2 +@pytest.mark.skipif(platform.python_implementation() != 'CPython', + reason='Requires CPython') def test_quiver_key_memory_leak(): fig, ax = plt.subplots() From 7100b3991f0eae591e4c19a0ac22ba261f979737 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Fri, 13 Aug 2021 23:08:29 -0400 Subject: [PATCH 3/8] Fix savefig extra argument warning on PyPy. PyPy adds an additional frame for `functools.wraps`, with `_functools` as the name, which causes the code to decide we're no longer in Matplotlib code too early, and not find `print_figure`. This causes warnings about unexpected arguments. --- lib/matplotlib/backend_bases.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py index d7be356d66d2..00e99c9f073e 100644 --- a/lib/matplotlib/backend_bases.py +++ b/lib/matplotlib/backend_bases.py @@ -1618,14 +1618,20 @@ def wrapper(*args, **kwargs): if frame is None: # when called in embedded context may hit frame is None. break + # Work around sphinx-gallery not setting __name__. + frame_name = frame.f_globals.get('__name__', '') if re.match(r'\A(matplotlib|mpl_toolkits)(\Z|\.(?!tests\.))', - # Work around sphinx-gallery not setting __name__. - frame.f_globals.get('__name__', '')): - if public_api.match(frame.f_code.co_name): - name = frame.f_code.co_name + frame_name): + name = frame.f_code.co_name + if public_api.match(name): if name in ('print_figure', '_no_output_draw'): seen_print_figure = True + elif frame_name == '_functools': + # PyPy adds an extra frame without module prefix for this + # functools wrapper, which we ignore to assume we're still in + # Matplotlib code. + continue else: break From 8c3dbd76181c470b3e392f5c4fdc2e27aa6b12a7 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Fri, 13 Aug 2021 23:27:21 -0400 Subject: [PATCH 4/8] Fix docstrings on optimization level 2 in PyPy. In CPython, `inspect.getdoc` returns some default string for `__init__`, but on PyPy, it returns None, when using `-OO`. https://foss.heptapod.net/pypy/pypy/-/issues/3534 --- lib/matplotlib/patches.py | 3 ++- lib/matplotlib/scale.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/patches.py b/lib/matplotlib/patches.py index 3fbf4ee245f7..5e053cb364a3 100644 --- a/lib/matplotlib/patches.py +++ b/lib/matplotlib/patches.py @@ -1451,7 +1451,8 @@ def _make_verts(self): docstring.interpd.update( - FancyArrow="\n".join(inspect.getdoc(FancyArrow.__init__).splitlines()[2:])) + FancyArrow="\n".join( + (inspect.getdoc(FancyArrow.__init__) or "").splitlines()[2:])) class CirclePolygon(RegularPolygon): diff --git a/lib/matplotlib/scale.py b/lib/matplotlib/scale.py index 180906106747..306a60aa4e66 100644 --- a/lib/matplotlib/scale.py +++ b/lib/matplotlib/scale.py @@ -620,10 +620,11 @@ def _get_scale_docs(): """ docs = [] for name, scale_class in _scale_mapping.items(): + docstring = inspect.getdoc(scale_class.__init__) or "" docs.extend([ f" {name!r}", "", - textwrap.indent(inspect.getdoc(scale_class.__init__), " " * 8), + textwrap.indent(docstring, " " * 8), "" ]) return "\n".join(docs) From f809ee58c19099b92c39c9734d30843f0b1e6926 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Sat, 14 Aug 2021 00:18:16 -0400 Subject: [PATCH 5/8] Skip Tkinter tests with threading on PyPy. It does not support calls to Tkinter from different threads: https://foss.heptapod.net/pypy/pypy/-/issues/1929 --- lib/matplotlib/tests/test_backend_tk.py | 4 ++++ lib/matplotlib/tests/test_backends_interactive.py | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/lib/matplotlib/tests/test_backend_tk.py b/lib/matplotlib/tests/test_backend_tk.py index 771d5193e397..5218d2c776a3 100644 --- a/lib/matplotlib/tests/test_backend_tk.py +++ b/lib/matplotlib/tests/test_backend_tk.py @@ -1,6 +1,7 @@ import functools import inspect import os +import platform import re import subprocess import sys @@ -111,6 +112,9 @@ def legitimate_quit(): print("success") +@pytest.mark.skipif(platform.python_implementation() != 'CPython', + reason='PyPy does not support Tkinter threading: ' + 'https://foss.heptapod.net/pypy/pypy/-/issues/1929') @pytest.mark.backend('TkAgg', skip_on_importerror=True) @pytest.mark.flaky(reruns=3) @_isolated_tk_test(success_count=1) diff --git a/lib/matplotlib/tests/test_backends_interactive.py b/lib/matplotlib/tests/test_backends_interactive.py index d3e04e986b0c..36ec3bfc83c3 100644 --- a/lib/matplotlib/tests/test_backends_interactive.py +++ b/lib/matplotlib/tests/test_backends_interactive.py @@ -3,6 +3,7 @@ import inspect import json import os +import platform import signal import subprocess import sys @@ -220,6 +221,12 @@ def _test_thread_impl(): elif param.values[0].get("QT_API") == "PySide2": param.marks.append( pytest.mark.xfail(raises=subprocess.CalledProcessError)) + elif backend == "tkagg" and platform.python_implementation() != 'CPython': + param.marks.append( + pytest.mark.xfail( + reason='PyPy does not support Tkinter threading: ' + 'https://foss.heptapod.net/pypy/pypy/-/issues/1929', + strict=True)) @pytest.mark.parametrize("env", _thread_safe_backends) From 30cd364f7137c42c4092650e58afa6ea77e59074 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Sat, 14 Aug 2021 02:13:13 -0400 Subject: [PATCH 6/8] Force GC run in tests that want clean up. Some of these worked (by fluke?) in CPython, but didn't work in PyPy. The NumPy function ensures GC is run enough times on PyPy to be really cleared. --- lib/matplotlib/tests/test_animation.py | 7 ++++--- lib/matplotlib/tests/test_cbook.py | 2 ++ lib/matplotlib/tests/test_style.py | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py index fbd984f70543..f077e7136bef 100644 --- a/lib/matplotlib/tests/test_animation.py +++ b/lib/matplotlib/tests/test_animation.py @@ -1,4 +1,3 @@ -import gc import os from pathlib import Path import subprocess @@ -90,7 +89,7 @@ def test_animation_delete(anim): anim = animation.FuncAnimation(**anim) with pytest.warns(Warning, match='Animation was deleted'): del anim - gc.collect() + np.testing.break_cycles() def test_movie_writer_dpi_default(): @@ -214,7 +213,8 @@ def test_animation_repr_html(writer, html, want, anim): if want is None: assert html is None with pytest.warns(UserWarning): - del anim # Animtion was never run, so will warn on cleanup. + del anim # Animation was never run, so will warn on cleanup. + np.testing.break_cycles() else: assert want in html @@ -324,6 +324,7 @@ def frames_generator(): writer = NullMovieWriter() anim.save('unused.null', writer=writer) assert len(frames_generated) == 5 + np.testing.break_cycles() for f in frames_generated: # If cache_frame_data is True, then the weakref should be alive; # if cache_frame_data is False, then the weakref should be dead (None). diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index 474ea1a41d93..88fac2dee435 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -198,11 +198,13 @@ def count(self): return count1 def is_empty(self): + np.testing.break_cycles() assert self.callbacks._func_cid_map == {} assert self.callbacks.callbacks == {} assert self.callbacks._pickled_cids == set() def is_not_empty(self): + np.testing.break_cycles() assert self.callbacks._func_cid_map != {} assert self.callbacks.callbacks != {} diff --git a/lib/matplotlib/tests/test_style.py b/lib/matplotlib/tests/test_style.py index aea09462b197..5ed115abb68f 100644 --- a/lib/matplotlib/tests/test_style.py +++ b/lib/matplotlib/tests/test_style.py @@ -1,9 +1,9 @@ from contextlib import contextmanager -import gc from pathlib import Path from tempfile import TemporaryDirectory import sys +import numpy as np import pytest import matplotlib as mpl @@ -165,7 +165,7 @@ def test_xkcd_no_cm(): assert mpl.rcParams["path.sketch"] is None plt.xkcd() assert mpl.rcParams["path.sketch"] == (1, 100, 2) - gc.collect() + np.testing.break_cycles() assert mpl.rcParams["path.sketch"] == (1, 100, 2) From 755cc760737d97a736a2b275e5437936c9f857d9 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 16 Aug 2021 23:36:56 -0400 Subject: [PATCH 7/8] Update to latest cibuildwheel. This is necessary to get the latest PyPy, which implements `METH_FASTCALL`. --- .github/workflows/cibuildwheel.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cibuildwheel.yml b/.github/workflows/cibuildwheel.yml index 6cbec141f7d3..84951cac4784 100644 --- a/.github/workflows/cibuildwheel.yml +++ b/.github/workflows/cibuildwheel.yml @@ -47,7 +47,7 @@ jobs: - name: Install cibuildwheel run: | - python -m pip install cibuildwheel==1.9.0 + python -m pip install cibuildwheel==2.1.1 - name: Build minimum NumPy for aarch64 if: matrix.cibw_archs == 'aarch64' && steps.numpy-cache.outputs.cache-hit != 'true' From e9fbe9f58b083e390205e1ad8a6f07128bde7bba Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 18 Aug 2021 02:29:41 -0400 Subject: [PATCH 8/8] Work around leak of `catch_warnings` in PyPy. https://foss.heptapod.net/pypy/pypy/-/issues/3536 --- lib/matplotlib/tests/test_animation.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/matplotlib/tests/test_animation.py b/lib/matplotlib/tests/test_animation.py index f077e7136bef..a7ba927494b1 100644 --- a/lib/matplotlib/tests/test_animation.py +++ b/lib/matplotlib/tests/test_animation.py @@ -1,5 +1,6 @@ import os from pathlib import Path +import platform import subprocess import sys import weakref @@ -86,6 +87,11 @@ def test_null_movie_writer(anim): @pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim']) def test_animation_delete(anim): + if platform.python_implementation() == 'PyPy': + # Something in the test setup fixture lingers around into the test and + # breaks pytest.warns on PyPy. This garbage collection fixes it. + # https://foss.heptapod.net/pypy/pypy/-/issues/3536 + np.testing.break_cycles() anim = animation.FuncAnimation(**anim) with pytest.warns(Warning, match='Animation was deleted'): del anim @@ -200,6 +206,11 @@ def test_save_animation_smoketest(tmpdir, writer, frame_format, output, anim): ]) @pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim']) def test_animation_repr_html(writer, html, want, anim): + if platform.python_implementation() == 'PyPy': + # Something in the test setup fixture lingers around into the test and + # breaks pytest.warns on PyPy. This garbage collection fixes it. + # https://foss.heptapod.net/pypy/pypy/-/issues/3536 + np.testing.break_cycles() if (writer == 'imagemagick' and html == 'html5' # ImageMagick delegates to ffmpeg for this format. and not animation.FFMpegWriter.isAvailable()):