BUG: tri: prevent Triangulation from modifying specified input

[pv]

triplot(x, y, simplex) should not modify the simplex array as a side effect.

[ianthomas23]

@pv: you need to argue the case for this change. It is not clear if this is a bug - no promise is made not to modify the triangles array. Admittedly the documentation doesn't say that that the array may be modified, but it did until PR #1494 less than a month ago. Perhaps all that is needed is better documentation.

[pv]

I'd argue that triplot modifying its input argument is very unexpected behavior, and it does break things. Plotting something should never change the data provided. (Of course, you can argue that the right place to make the copy is in triplot et al.)

Consider this:

>>> points=[(0,0),(0,1.1),(1,0),(1,1)]
>>> from scipy.spatial import Delaunay
>>> tri = Delaunay(points)
>>> tri.vertices
array([[3, 2, 0],
       [3, 1, 0]], dtype=int32)
>>> plt.triplot(tri.points[:,0], tri.points[:,1], tri.vertices)
>>> tri.vertices
array([[3, 0, 2],
       [3, 1, 0]], dtype=int32)

Now, the Qhull invariant of how vertices relate to the neighborhood structure is broken. This causes e.g. the simplex finding code to fail spuriously later on.

[WeatherGod]

Agreed that this is a bug. While it isn't explicitly documented, and we don't explicitly test for this, it is a general rule that we don't modify input arguments in-place. We are only plotting. In general, we use the inputs to the plotting functions as-is to prevent unneeded copies, but if changes are needed, then we copy.

[ianthomas23]

OK, if it is a bug the changes need to be made based on the maintenance branch, not master, as specified in the pull request check list.

There are ramifications for existing uses of this code. @pv: the analysis of this bug should include information about under what circumstances the triangles array is modified (i.e. is it always?) and potential detrimental effects on uses cases other than the one you are interested it. Without this no developer is going to commit the change. It would have been better to open an issue to discuss it and consider the pros and cons of possible changes before writing a fix; this would also have provided an issue number for the new test which would be good.

[pv]

Very well: #1584

[pv]

Ok, rebased against v1.2.x, replaced simplices -> triangles, and reopened.

[ianthomas23]

Fix applied to both v1.2.x and master. Thanks @pv.