Bug in contour x/y values filtering in mpl_toolkits/basemap/__init__.py ?

[jypeter]

I have been trying to plot unstructured data with contour and contourf, using the tri=True parameter. It works fine with contourf, but I get a huge traceback when using contour on the same data.

[...]
  File "/home/share/unix_files/cdat/versions/cdat_install_uv-2.1.0_x86_64_gcc4_13/lib/python2.7/site-packages/matplotlib-1.4.0-py2.7-linux-x86_64.egg/matplotlib/tri/triangulation.py", line 55, in __init__
    self.triangles, self._neighbors = _qhull.delaunay(x, y)
ValueError: x and y arrays must have a length of at least 3

After spending some time in the python debugger, I have found out that indeed the x and y coordinate arrays passed to the triangulation function had a zero size (and therefore less than 3 elements) because they were empty! More digging helped me find a very suspicious way of filtering the x and y values in the contour(self,x,y,data,args,*kwargs) function

                # for unstructured grids, toss out points outside
                # projection limb (don't use those points in triangulation).
[...]
                mask = np.logical_or(x<self.xmin,y<self.xmin) +\
                       np.logical_or(x>self.xmax,y>self.xmax)
                x = np.compress(mask,x)
                y = np.compress(mask,y)

Why would y be compared to the values of xmin and xmax instead of ymin and ymax, and is the logical combination ok???

I think we want to keep the values where: xmin<=x<=xmax AND ymin<=y<=ymax. And the mask has to be True where we want to keep the value, in np.compress! This is a bit misleading because it works in the opposite way that masks work in np.ma ...

The contour function works fine if I replace the mask definition above with

                mask = np.logical_and(np.logical_and(x>=self.xmin, x<=self.xmax),
                                      np.logical_and(y>=self.ymin, y<=self.ymax))

Can somebody review this? And it may be wise to use a slightly less misleading name for the mask variable. Maybe replace mask with select_xy_ok?

[WeatherGod]

Wow, that looks pretty bad. Yes, indeed the y comparisons should be against
self.ymin and self.ymax. And the mask being passed to np.compress() should
be True for coordinates within the domain, rather than how it seems to be
used here. Admitted, I am not entirely convinced that we should even be
clipping, but git blame says it was added to deal with some proj4 bug at
the time.

As for the name of the variable, it is a mask, just a negative mask, so I
am not concerned about changing the name of the internal variable at the
moment. Would you like to put together a PR with this fix?

On Wed, Feb 17, 2016 at 11:45 AM, Jean-Yves Peterschmitt <
notifications@github.com> wrote:

I have been trying to plot unstructured data with contour and contourf,
using the tri=True parameter. It works fine with contourf, but I get a
huge traceback when using contour on the same data.

[...]
File "/home/share/unix_files/cdat/versions/cdat_install_uv-2.1.0_x86_64_gcc4_13/lib/python2.7/site-packages/matplotlib-1.4.0-py2.7-linux-x86_64.egg/matplotlib/tri/triangulation.py", line 55, in init
self.triangles, self._neighbors = _qhull.delaunay(x, y)
ValueError: x and y arrays must have a length of at least 3

After spending some time in the python debugger, I have found out that
indeed the x and y coordinate arrays passed to the triangulation function
had a zero size (and therefore less than 3 elements) because they were
empty! More digging helped me find a very suspicious way of filtering the x
and y values in the contour(self,x,y,data,args,*kwargs) function

            # for unstructured grids, toss out points outside
            # projection limb (don't use those points in triangulation).

[...]
mask = np.logical_or(x<self.xmin,y<self.xmin) +
np.logical_or(x>self.xmax,y>self.xmax)
x = np.compress(mask,x)
y = np.compress(mask,y)

Why would y be compared to the values of xmin and xmax instead of ymin and
ymax, and is the logical combination ok???

I think we want to keep the values where: xmin<=x<=xmax AND
ymin<=y<=ymax. And the mask has to be True where we want to keep the
value, in np.compress! This is a bit misleading because it works in the
opposite way that masks work in np.ma ...

The contour function works fine if I replace the mask definition above with

            mask = np.logical_and(np.logical_and(x>=self.xmin, x<=self.xmax),
                                  np.logical_and(y>=self.ymin, y<=self.ymax))

Can somebody review this? And it may be wise to use a slightly less
misleading name for the mask variable. Maybe replace mask with
select_xy_ok?

—
Reply to this email directly or view it on GitHub
#265.

[jypeter]

I'm not familiar enough with the PR process, and I have already made the change in the python distribution I use, so somebody more knowledgeable should take care of it.

Something needs to be done about this obvious bug, but someone who knows basemap (better than me) should compare the code of contour and contourf, because contourf does not handle the mask for unstructured data the same way at all. For contour you have:

        If x or y are outside projection limb (i.e. they have values > 1.e20),
        the corresponing data elements will be masked.
[...]
                mask = np.logical_or(x<1.e20,y<1.e20)
                x = np.compress(mask,x)
                y = np.compress(mask,y)

Besides, the way the mask is handled for structured data is not the same either, for contour and contourf, so it may be interesting to check this as well

Maybe this needs to be a 2 step process: first correct the obvious bug with the patch I have submitted, so this is taken care of, and then spend more time checking if the masking can be done in a consistent way for contour and contourf, which probably requires more thinking

[jenshnielsen]

@WeatherGod Shouldn't this be raised as a bug against basemap?

[WeatherGod]

It is a bug filed against basemap...

On Thu, Feb 18, 2016 at 3:58 AM, Jens Hedegaard Nielsen <
notifications@github.com> wrote:

@WeatherGod https://github.com/WeatherGod Shouldn't this be raised as a
bug against basemap?

—
Reply to this email directly or view it on GitHub
#265 (comment).

[jypeter]

What do you mean by that? The bug was already listed somewhere? Or it is now?

[micahcochran]

@jenshnielsen Do you mean that a label needs to be added to this issue that says this issue a bug report?

[WeatherGod]

Don't worry, I think Jens got a little confused. Often, people file bug
reports about basemap in the matplotlib issue tracker. You did it correctly.

On Thu, Feb 18, 2016 at 9:43 AM, Jean-Yves Peterschmitt <
notifications@github.com> wrote:

What do you mean by that? The bug was already listed somewhere? Or it is
now?

—
Reply to this email directly or view it on GitHub
#265 (comment).

[jenshnielsen]

Sorry I must have confused this with something else

[bhazelton]

I've just run into this same bug, has it been fixed somewhere? I see that the issue is still open...

As a side note, pcolor also works (as does contourf), only contour seems to be broken.

[jypeter]

Wow, I had completely forgotten about this! I don't use basemap/matplotlib that much and can't take care of this. I hope someone else can!

[maxpittore]

Hallo,
it seems the bug is still there. Any news ? It is a pity contour cannot be used on unstructured lists.
Besides this, thank you for your efforts, very nice package !

[guidocioni]

Is there any update on this? I cannot plot unstructured data with basemap contour and using tricontour from matplotlib.pyplot gives me an Error in qhull Delaunay triangulation.