Skip to content

pypy failures #19160

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

Closed
39 tasks done
tacaswell opened this issue Dec 21, 2020 · 9 comments · Fixed by #20848
Closed
39 tasks done

pypy failures #19160

tacaswell opened this issue Dec 21, 2020 · 9 comments · Fixed by #20848
Assignees
Milestone

Comments

@tacaswell
Copy link
Member

tacaswell commented Dec 21, 2020

Going through the logs from @mattip 's logs in #19154 I think the failures we are seeing can be classified as below.

finalizer / weakrefs different in pypy

We probably should just detect and skip pypy on these.

  • FAILED lib/matplotlib/tests/test_animation.py::test_animation_delete[anim0]
  • FAILED lib/matplotlib/tests/test_animation.py::test_animation_repr_html[anim0-none-None-imagemagick]
  • FAILED lib/matplotlib/tests/test_animation.py::test_animation_repr_html[anim0-none-None-ffmpeg]
  • FAILED lib/matplotlib/tests/test_animation.py::test_funcanimation_cache_frame_data[False]
  • FAILED lib/matplotlib/tests/test_cbook.py::Test_callback_registry::test_callback_complete

pypy does not implement a working sys.getsizeof

Skip this test on pypy, we are only using it to make sure that the output of rendering makes sense.

  • FAILED lib/matplotlib/tests/test_backend_pdf.py::test_multipage_properfinalize

stack walking different?

The failures due to raising an unexpected warning are a bit more concerning. We are doing some frame walking / inspection to sort out if we should warn or not in

@functools.wraps(func)
def wrapper(*args, **kwargs):
name = 'savefig' # Reasonable default guess.
public_api = re.compile(r'^savefig|print_[A-Za-z0-9]+$')
seen_print_figure = False
for frame, line in traceback.walk_stack(None):
if frame is None:
# when called in embedded context may hit frame is None.
break
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
if name == 'print_figure':
seen_print_figure = True
else:
break
accepted_kwargs = {*old_sig.parameters, *extra_kwargs}
if seen_print_figure:
for kw in ['dpi', 'facecolor', 'edgecolor', 'orientation',
'bbox_inches_restore']:
# Ignore keyword arguments that are passed in by print_figure
# for the use of other renderers.
if kw not in accepted_kwargs:
kwargs.pop(kw, None)
for arg in list(kwargs):
if arg in accepted_kwargs:
continue
cbook.warn_deprecated(
'3.3', name=name,
message='%(name)s() got unexpected keyword argument "'
+ arg + '" which is no longer supported as of '
'%(since)s and will become an error '
'%(removal)s')
kwargs.pop(arg)
return func(*args, **kwargs)
return wrapper
I assume this is not working on pypy because we are relying on some cpython implementation detail?

If someone who understands pypy can fix that to work on both that would be best, but if not we should detect if we are on pypy and fall back to never warning. It is operating in a slightly degraded mode (users may not get warnings they should see), but the status quo is going to warn for things the user can not do anything about (I think every time we use tightlayout of bbox_inches='tight').

This is probably the only failures I would hold 3.4 over.

  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight[png]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight[svg]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_suptile_legend[pdf]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_clipping[png]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_clipping[svg]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_raster[pdf]
  • FAILED lib/matplotlib/tests/test_backend_pgf.py::test_bbox_inches_tight - mat...
  • FAILED lib/matplotlib/tests/test_backend_pgf.py::test_mixedmode[pdf] - matplo...
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight[pdf]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_suptile_legend[png]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_suptile_legend[svg]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_clipping[pdf]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_raster[png]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_bbox_inches_tight_raster[svg]
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_tight_pcolorfast - matpl...
  • FAILED lib/matplotlib/tests/test_figure.py::test_tightbbox_box_aspect[svg] - ...
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_only_on_non_finite_bbox
  • FAILED lib/matplotlib/tests/test_bbox_tight.py::test_noop_tight_bbox - matplo...
  • FAILED lib/matplotlib/tests/test_offsetbox.py::test_annotationbbox_extents - ...
  • FAILED lib/mpl_toolkits/tests/test_axes_grid1.py::test_image_grid[png] - matp...

segfaults in tk

We should either track this down or just skip the tests.

  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolbar2-tkagg]
  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolmanager-gtk3agg]
  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolmanager-tkagg]

subprocess hung

We should either track this down or just skip the tests.

  • FAILED lib/matplotlib/tests/test_backend_tk.py::test_never_update - Failed: S...
  • FAILED lib/matplotlib/tests/test_backend_tk.py::test_figuremanager_cleans_own_mainloop
  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolbar2-gtk3agg]
  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[gtk3agg]
  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[tkagg]

not matching numpy?

These both look like they are upstream bugs in pypy? We should probably just skip these.

  • FAILED lib/matplotlib/tests/test_cbook.py::test_step_fails[args1] - SystemErr...
  • FAILED lib/matplotlib/tests/test_matplotlib.py::test_importable_with__OO - As...

checking refcounts

These tests do not make sense for pypy, we should skip them.

  • FAILED lib/matplotlib/tests/test_quiver.py::test_quiver_memory_leak - Attribu...
  • FAILED lib/matplotlib/tests/test_quiver.py::test_quiver_key_memory_leak - Att...

sockets not working

The comments around this suggest we never got this working on azure (which I assume is what is hosting GH actions) so we should sort out why the code to not run these tests in that case is not being triggered here.

  • FAILED lib/matplotlib/tests/test_backends_interactive.py::test_webagg - Faile...
@anntzer
Copy link
Contributor

anntzer commented Dec 21, 2020

I don't have an easy way to check, but I guess the animation failures can be fixed by just adding a call to gc.collect() after del anim or before for f in frames_generated?

@tacaswell tacaswell modified the milestones: v3.4.0, v3.4.1 Feb 17, 2021
@QuLogic QuLogic modified the milestones: v3.4.1, v3.4.2 Mar 30, 2021
@QuLogic QuLogic modified the milestones: v3.4.2, v3.4.3 May 5, 2021
@QuLogic QuLogic modified the milestones: v3.4.3, v3.5.0 Aug 11, 2021
@QuLogic
Copy link
Member

QuLogic commented Aug 14, 2021

I don't know how to fix these animation warning tests, because adding gc.collect does not help:

FAILED lib/matplotlib/tests/test_animation.py::test_animation_delete[anim0] - Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) was emitted. The list of emitted warnings is: [].
FAILED lib/matplotlib/tests/test_animation.py::test_animation_repr_html[anim0-none-None-ffmpeg] - Failed: DID NOT WARN. No warnings of type (<class 'UserWarning'>,) was emitted. The list of emitted warnings is: [].
FAILED lib/matplotlib/tests/test_animation.py::test_animation_repr_html[anim0-none-None-imagemagick] - Failed: DID NOT WARN. No warnings of type (<class 'UserWarning'>,) was emitted. The list of emitted warnings is: [].

Clearly, the warning is happening, because it is printed out to stderr:

Exception ignored in: <bound method Animation.__del__ of <matplotlib.animation.FuncAnimation object at 0x0000000004839210>>
Traceback (most recent call last):
  File ".../lib/matplotlib/animation.py", line 890, in __del__
    'Animation was deleted without rendering anything. This is '
UserWarning: Animation was deleted without rendering anything. This is most likely unintended. To prevent deletion, assign the Animation to a variable that exists for as long as you need the Animation.

but it does not seem to get captures by pytest.warns.

Otherwise, I've managed to fixed all the other failures (except maybe Qt, since I couldn't install the deps).

@QuLogic QuLogic self-assigned this Aug 14, 2021
@mattip
Copy link
Contributor

mattip commented Aug 14, 2021

I think you need to call gc.collect() at least 3 times in a loop. NumPy has a break_cycles function for this

    gc.collect()
    if IS_PYPY:
        # interpreter runs now, to call deleted objects' __del__ methods
        gc.collect()
        # two more, just to make sure
        gc.collect()
        gc.collect()

@QuLogic
Copy link
Member

QuLogic commented Aug 16, 2021

Thanks, that's even public too. However, it doesn't work, even calling it twice (so gc.collect 8 times), the Animation is not cleaned up in the pytest.warns (but it is when the test exits).

@QuLogic
Copy link
Member

QuLogic commented Aug 16, 2021

Or maybe the Animation is cleaned up there, but pytest is unable to capture the warning that's raised in __del__?

@QuLogic
Copy link
Member

QuLogic commented Aug 17, 2021

So I added a weakref on the object, and checked it before and after break_cycles, and the object is gone. Looking at pytest.warns, it's really just a wrapper around warnings.catch_warnings, so I changed the test to:

def test_animation_repr_html(writer, html, want, anim):                                                                                                                                                                                                                                                     
    if (writer == 'imagemagick' and html == 'html5'                                                                                                                                                                                                                                                         
            # ImageMagick delegates to ffmpeg for this format.                                                                                                                                                                                                                                              
            and not animation.FFMpegWriter.isAvailable()):                                                                                                                                                                                                                                                  
        pytest.skip('Requires FFMpeg')                                                                                                                                                                                                                                                                      
    # create here rather than in the fixture otherwise we get __del__ warnings                                                                                                                                                                                                                              
    # about producing no output                                                                                                                                                                                                                                                                             
    anim = animation.FuncAnimation(**anim)                                                                                                                                                                                                                                                                  
    with plt.rc_context({'animation.writer': writer,                                                                                                                                                                                                                                                        
                         'animation.html': html}):                                                                                                                                                                                                                                                          
        html = anim._repr_html_()                                                                                                                                                                                                                                                                           
    if want is None:                                                                                                                                                                                                                                                                                        
        assert html is None                                                                                                                                                                                                                                                                                 
        import warnings                                                                                                                                                                                                                                                                                     
        with warnings.catch_warnings(record=True) as w:                                                                                                                                                                                                                                                     
            warnings.simplefilter("always")                                                                                                                                                                                                                                                                 
            wranim = weakref.ref(anim)                      
            del anim  # Animation was never run, so will warn on cleanup.                                                                                                                                                                                                                                   
            print(wranim())                                                                                                                                                                                                                                                                                 
            np.testing.break_cycles()                                                                                                                                                                                                                                                                       
            print(wranim())                                                                                                                                                                                                                                                                                 
            assert len(w) == 1                                                                                                                                                                                                                                                                              
            assert issubclass(w[0], UserWarning)                                                                                                                                                                                                                                                            
    else:                                                                                                                                                                                                                                                                                                   
        assert want in html                                                                                                                                                                                                                                                                                 

and still no warnings are recorded, but are printed to stderr as an 'ignored Exception'. It seems that PyPy does not support capturing warnings emitted in __del__.

@QuLogic
Copy link
Member

QuLogic commented Aug 17, 2021

The closest I can get is to mark the test with @pytest.mark.filterwarnings('always'), which converts the warning from an unhandled exception (because we set warnings to error) in a destructor to a warning captured at the exit of the test, but the change to warning filtering within the test does nothing.

@mattip
Copy link
Contributor

mattip commented Aug 17, 2021

It might be best just to skip this test on PyPy so you don't spend too much time with it. As I understand it, the warning is meant to inform the user that the animation has never been run, which could be considered a bit of an edge case. PyPy does catch warnings in other cases. Just yesterday I added a single gc.collect() to some lib-python tests to get a warning printed when the object is destroyed, so I am not sure what is going on here specifically.

@QuLogic
Copy link
Member

QuLogic commented Aug 17, 2021

so you don't spend too much time with it.

Too late; I spent yesterday whittling it down from tests, fixtures, and Matplotlib code to a small script: https://foss.heptapod.net/pypy/pypy/-/issues/3536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants