Skip to content

plt.bar with nan input fails rendering in notebook using 3.3.0rc1 #17868

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.

Sign up for GitHub

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

Jump to bottom
Closed
MaozGelbart opened this issue Jul 9, 2020 · 6 comments · Fixed by #17885
Closed

plt.bar with nan input fails rendering in notebook using 3.3.0rc1 #17868

MaozGelbart opened this issue Jul 9, 2020 · 6 comments · Fixed by #17885
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. status: confirmed bug topic: path handling
Milestone
v3.3.0

Comments

@MaozGelbart
Copy link
Contributor

Bug report

Bug summary

A previously-working code (that gets to plot well using other backends or matplotlib==3.2.2) fails to render a plot on Jupyter notebook when using matplotlib==3.3.0rc1. This has to do with np.nan input to plt.bar.

Code for reproduction

import matplotlib.pyplot as plt
import matplotlib as mpl
import numpy as np
%matplotlib inline

x = range(2)
heights = np.asarray([np.nan, 4])

plt.bar(x, heights)

Actual outcome

<BarContainer object of 2 artists>
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
~/.local/lib/python3.8/site-packages/IPython/core/formatters.py in __call__(self, obj)
    339                 pass
    340             else:
--> 341                 return printer(obj)
    342             # Finally look for special method names
    343             method = get_real_method(obj, self.print_method)

~/.local/lib/python3.8/site-packages/IPython/core/pylabtools.py in <lambda>(fig)
    246 
    247     if 'png' in formats:
--> 248         png_formatter.for_type(Figure, lambda fig: print_figure(fig, 'png', **kwargs))
    249     if 'retina' in formats or 'png2x' in formats:
    250         png_formatter.for_type(Figure, lambda fig: retina_figure(fig, **kwargs))

~/.local/lib/python3.8/site-packages/IPython/core/pylabtools.py in print_figure(fig, fmt, bbox_inches, **kwargs)
    130         FigureCanvasBase(fig)
    131 
--> 132     fig.canvas.print_figure(bytes_io, **kw)
    133     data = bytes_io.getvalue()
    134     if fmt == 'svg':

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/backend_bases.py in print_figure(self, filename, dpi, facecolor, edgecolor, orientation, format, bbox_inches, pad_inches, bbox_extra_artists, backend, **kwargs)
   2186                         self.figure.draw(renderer)
   2187 
-> 2188                     bbox_inches = self.figure.get_tightbbox(
   2189                         renderer, bbox_extra_artists=bbox_extra_artists)
   2190                     if pad_inches is None:

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/figure.py in get_tightbbox(self, renderer, bbox_extra_artists)
   2500 
   2501         for a in artists:
-> 2502             bbox = a.get_tightbbox(renderer)
   2503             if bbox is not None and (bbox.width != 0 or bbox.height != 0):
   2504                 bb.append(bbox)

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/artist.py in get_tightbbox(self, renderer)
    276             The enclosing bounding box (in figure pixel coordinates).
    277         """
--> 278         bbox = self.get_window_extent(renderer)
    279         if self.get_clip_on():
    280             clip_box = self.get_clip_box()

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/patches.py in get_window_extent(self, renderer)
    596 
    597     def get_window_extent(self, renderer=None):
--> 598         return self.get_path().get_extents(self.get_transform())
    599 
    600     def _convert_xy_units(self, xy):

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/path.py in get_extents(self, transform, **kwargs)
    590             self = transform.transform_path(self)
    591         bbox = Bbox.null()
--> 592         for curve, code in self.iter_bezier(**kwargs):
    593             # places where the derivative is zero can be extrema
    594             _, dzeros = curve.axis_aligned_extrema()

~/miniconda/envs/seaborn_devenv/lib/python3.8/site-packages/matplotlib/path.py in iter_bezier(self, **kwargs)
    442             if first_vert is None:
    443                 if code != Path.MOVETO:
--> 444                     raise ValueError("Malformed path, must start with MOVETO.")
    445             if code == Path.MOVETO:  # a point is like "CURVE1"
    446                 first_vert = verts

ValueError: Malformed path, must start with MOVETO.

<Figure size 432x288 with 1 Axes>

Expected outcome

3.2.2:
Screen Shot 2020-07-09 at 16 15 04

Matplotlib version

  • Operating system: macOS
  • Matplotlib version: 3.3.0rc1
  • Matplotlib backend (print(matplotlib.get_backend())): 'module://ipykernel.pylab.backend_inline'
  • Python version: 3.8.3
  • Jupyter version (if applicable): notebook 6.0.3, IPython 7.15.0
  • Other libraries:

matplotlib installed using pip
python installed from conda-forge
@dstansby dstansby added status: confirmed bug Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Jul 9, 2020
@dstansby
Copy link
Member

dstansby commented Jul 9, 2020

I can reproduce this with the current master branch in a jupyter notebook.

@dstansby dstansby added this to the v3.3.0 milestone Jul 9, 2020
@dstansby
Copy link
Member

dstansby commented Jul 9, 2020

Bisects to 95a530c

@dstansby
Copy link
Member

dstansby commented Jul 9, 2020

Some more digging, and I think the changes in #16832 are exposing a pre-existing bug in _path.cleanup_path. The code path to this error includes _path.cleanup_path being called with remove_nans=True on

Path(array([[nan, nan],
       [nan, nan],
       [nan, nan],
       [nan, nan],
       [nan, nan]]), array([ 1,  2,  2,  2, 79], dtype=uint8))

and it is returning

[[nan nan]
 [ 0.  0.]] [79  0]

which is odd, I would expect it to return something empty since the input was all NaNs. This then propagates to a point where matplotlib complains, because this path does not start with a MOVETO (ie. 1) code.

@QuLogic
Copy link
Member

QuLogic commented Jul 9, 2020

cc @brunobeltran

@brunobeltran
Copy link
Contributor

This appears to simply be a missed corner case in my rewrite of get_extents (tbh, I thought I checked the all-NaN's case, sorry...). I'll push a fix with a new test in a minute!

@brunobeltran
Copy link
Contributor

brunobeltran commented Jul 11, 2020 •
edited
Loading

After some digging: in _path.h, the cleanup_path function removes NaN's using the algorithm in PathNanRemover's constructor. When this function encounters a NaN, it replaces that BezierSegment in the Path by a MOVETO (no-op) whose vertex is taken from the first control point of the next BezierSegment in the Path.

However, due to the way that PathNanRemover's logic is arranged, CLOSEPOLY commands are always passed thru as-is regardless of whether or non they are NaN (except in corner cases where there should not have been a CLOSEPOLY there in the first place, like coces=[1, 3, 79, 0])

Our C++ implementation of update_path_extents doesn't mind this, because it doesn't actually deal correctly with Path's that contain curves, so it can just go through the control points one by one and if it finds a CLOSEPOLY, it knows that by definition it has already seen that control point before, so it can just ignore it:

        if ((code & agg::path_cmd_end_poly) == agg::path_cmd_end_poly) {
            continue;
        }

However, the new Path.get_extents does correctly deal with arbitrary Paths, but to do so it relies on Path.cleaned to return a valid Path, (which it currently does not, per the above).

As pointed out by David, this is definitely a bug in Path.cleaned. I initially was tempted to just patch Path.iter_bezier to look out for CLOSEPOLY commands that contain a NaN explicitly and just skip them (hence my comment above), but I think the correct solution is to just fix PathNanRemover. Thankfully, the algorithm it uses is very straightforward so this shouldn't be too hard....

@brunobeltran brunobeltran mentioned this issue Jul 11, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. status: confirmed bug topic: path handling
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging a pull request may close this issue.

BF: ignore CLOSEPOLY after NaN in PathNanRemover brunobeltran/matplotlib
4 participants
@QuLogic @brunobeltran @dstansby @MaozGelbart