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

[pv]

triplot modifies its input simplex argument, as it orients the triangles anticlockwise. Plotting something should not change the data provided, as this is rather unexpected.

Example case:

>>> 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)

This e.g. breaks the Qhull invariant on how vertices relate to the neighborhood structure, so it would be useful not to do this.

[ianthomas23]

OK, here is my analysis of the problem.

There are two ways of using a Triangulation, you either specify the triangles yourself or let the code create a Delaunay triangulation for you. The bug only affects the former. The triangles are specified as an array of triangles, each of which is three point indices. Triangulation expects the triangles to be ordered in an anticlockwise manner; if they are ordered in a clockwise manner the indices of the incorrect triangles are reordered so that they are anticlockwise. As the triangles array is not copied before it is altered, this has the unwanted side effect of changing the user's triangles array.

The problem occurs only if a user specifies a triangles array containing one or more incorrectly-ordered (clockwise) triangles. If we fix this, there may be other users who are inadvertently relying on the Triangulation 'correcting' their triangles array and who now observe different behaviour. This is acceptable, but we need to be aware that some users may be confused by the change.

There are two options for a fix:

1. Triangulation always copies the triangles array, as in the attached PR.
2. Triangulation only copies the triangles array if it is going to modify it.

Option 1 is quick and easy, but for users using the 'correct' triangle order may result in an unnecessary array copy. Option 2 is more work, some in C++, but is not difficult. I am happy to do either, but what do other developers prefer? Is avoiding an unnecessary array copy worth the extra complexity of option 2?

[WeatherGod]

Could another option be to detect the clockwise orientation and consider it
an error? Possibly offering up a helper function in the tri module that
would correct the triangulation (with maybe a copy=[True|False]) flag so
that the user explicitly could fix their array?

[pv]

The plotting functions probably should still work with badly oriented arrays.

Note that a copy is silently made also if the input data type happens to be something else than int32 (for instance, the default Numpy int type on 64-bit platforms is int64). So users relying on this may be playing a bit of a dangerous game.

[ianthomas23]

@WeatherGod: I would rather not change the existing behaviour of accepting and correcting clockwise orientation as it has worked liked this since the tri-functions were added and changing it will have a large impact. Also, the choice of anticlockwise as the 'correct' orientation is somewhat arbitrary, so accepting either orientation is sensible.

[ianthomas23]

After consideration, I think we should go for the simplest option for the fix (item 1 above) as in @pv's original PR #1576.

@pv: I can't seem to reopen your PR. Perhaps, as the owner, you can do so. If not, could submit a new PR? Could you also change your use of 'simplices' to 'triangles', as this is consistent with other usage of the tri-functions? Then I will merge the changes into both the maintenance and master branches. If you can't or don't want to then I can create a new PR and merge it, but then your name won't be attached to the fix so you won't get the credit you are due for writing the fix.

[pv]

@ianthomas23: sure, I'll try to reopen it.

[ianthomas23]

PR #1576 applied to both v1.2.x and master.