Allow Paths to be marked as readonly #2010

Merged
merged 4 commits into from May 15, 2013
View
16 doc/api/api_changes.rst
@@ -15,6 +15,22 @@ For new features that were added to matplotlib, please see
Changes in 1.3.x
================
+* `Path` objects can now be marked as `readonly` by passing
+ `readonly=True` to its constructor. The built-in path singletons,
+ obtained through `Path.unit*` class methods return readonly paths.
+ If you have code that modified these, you will need to make a
+ deepcopy first, using either::
+
+ import copy
+ path = copy.deepcopy(Path.unit_circle())
+
+ # or
+
+ path = Path.unit_circle().deepcopy()
+
+ Deep copying a `Path` always creates an editable (i.e. non-readonly)
+ `Path`.
+
* The `font.*` rcParams now affect only text objects created after the
rcParam has been set, and will not retroactively affect already
existing text objects. This brings their behavior in line with most
View
1 lib/matplotlib/__init__.py
@@ -1193,6 +1193,7 @@ def tk_window_focus():
'matplotlib.tests.test_mathtext',
'matplotlib.tests.test_mlab',
'matplotlib.tests.test_patches',
+ 'matplotlib.tests.test_path',
'matplotlib.tests.test_patheffects',
'matplotlib.tests.test_pickle',
'matplotlib.tests.test_rcparams',
View
135 lib/matplotlib/path.py
@@ -84,7 +84,8 @@ class Path(object):
code_type = np.uint8
- def __init__(self, vertices, codes=None, _interpolation_steps=1, closed=False):
+ def __init__(self, vertices, codes=None, _interpolation_steps=1, closed=False,
+ readonly=False):
"""
Create a new path with the given vertices and codes.
@@ -109,6 +110,8 @@ def __init__(self, vertices, codes=None, _interpolation_steps=1, closed=False):
such as Polar, that this path should be linearly interpolated
immediately before drawing. This attribute is primarily an
implementation detail and is not intended for public use.
+
+ *readonly*, when True, makes the path immutable.
"""
if ma.isMaskedArray(vertices):
vertices = vertices.astype(np.float_).filled(np.nan)
@@ -130,14 +133,117 @@ def __init__(self, vertices, codes=None, _interpolation_steps=1, closed=False):
assert vertices.ndim == 2
assert vertices.shape[1] == 2
- self.should_simplify = (rcParams['path.simplify'] and
- (len(vertices) >= 128 and
- (codes is None or np.all(codes <= Path.LINETO))))
- self.simplify_threshold = rcParams['path.simplify_threshold']
- self.has_nonfinite = not np.isfinite(vertices).all()
- self.codes = codes
- self.vertices = vertices
+ self._vertices = vertices
+ self._codes = codes
self._interpolation_steps = _interpolation_steps
+ self._update_values()
+
+ if readonly:
+ self._vertices.flags.writeable = False
+ if self._codes is not None:
+ self._codes.flags.writeable = False
+ self._readonly = True
+ else:
+ self._readonly = False
+
+ def _update_values(self):
+ self._should_simplify = (
+ rcParams['path.simplify'] and
+ (len(self._vertices) >= 128 and
+ (self._codes is None or np.all(self._codes <= Path.LINETO))))
+ self._simplify_threshold = rcParams['path.simplify_threshold']
+ self._has_nonfinite = not np.isfinite(self._vertices).all()
+
+ @property
+ def vertices(self):
+ """
+ The list of vertices in the `Path` as an Nx2 numpy array.
+ """
+ return self._vertices
+
+ @vertices.setter
+ def vertices(self, vertices):
+ if self._readonly:
+ raise AttributeError("Can't set vertices on a readonly Path")
+ self._vertices = vertices
+ self._update_values()
+
+ @property
+ def codes(self):
+ """
+ The list of codes in the `Path` as a 1-D numpy array. Each
+ code is one of `STOP`, `MOVETO`, `LINETO`, `CURVE3`, `CURVE4`
+ or `CLOSEPOLY`. For codes that correspond to more than one
+ vertex (`CURVE3` and `CURVE4`), that code will be repeated so
+ that the length of `self.vertices` and `self.codes` is always
+ the same.
+ """
+ return self._codes
+
+ @codes.setter
+ def codes(self, codes):
+ if self._readonly:
+ raise AttributeError("Can't set codes on a readonly Path")
+ self._codes = codes
+ self._update_values()
+
+ @property
+ def simplify_threshold(self):
@pelson
pelson May 15, 2013

I'm not sure this property (and some of the others) really need to exist. The original attribute access was perfectly satisfactory - if you changed something that didn't make sense then you would expect it to blow up...
I guess what I'm saying is that I'm not really a big fan of protecting values in this way - if a user wants to fiddle with them, then they should be able to.

@mdboom
mdboom May 15, 2013

The nice thing about this approach is that when the vertices are updated, these values are updated as well, so they should always be correct. I think that's a good thing.

@pelson
pelson May 15, 2013

I think that's a good thing.

I agree, but there are always corner cases 😉

path.vertices[10:50] += 10

I'm happy with the read only protection for coding mistakes (such as mine) - but this is python: I don't think we need to worry about protecting the whole state of Path instances.

Either way, it's much of a muchness - and more important than whether it is a property or an actual attribute, I think there should be a oneline docstring for all of these.

@mdboom
mdboom May 15, 2013

I think I'll leave this as properties -- I think in most cases this is preferred.

Agreed about the docstrings.

I realise we can't protect much, but I think this was a particularly surprising one, mainly because I think users are surprised that Path.unit_circle() is a singleton. I could have also resolved this by having those return a copy, but that breaks some optimizations in markers and elsewhere where it tries to save the same path only once wherever possible.

+ """
+ The fraction of a pixel difference below which vertices will
+ be simplified out.
+ """
+ return self._simplify_threshold
+
+ @simplify_threshold.setter
+ def simplify_threshold(self, threshold):
+ self._simplify_threshold = threshold
+
+ @property
+ def has_nonfinite(self):
+ """
+ `True` if the vertices array has nonfinite values.
+ """
+ return self._has_nonfinite
+
+ @property
+ def should_simplify(self):
+ """
+ `True` if the vertices array should be simplified.
+ """
+ return self._should_simplify
+
+ @should_simplify.setter
+ def should_simplify(self, should_simplify):
+ self._should_simplify = should_simplify
+
+ @property
+ def readonly(self):
+ """
+ `True` if the `Path` is read-only.
+ """
+ return self._readonly
+
+ def __copy__(self):
+ """
+ Returns a shallow copy of the `Path`, which will share the
+ vertices and codes with the source `Path`.
+ """
+ import copy
+ return copy.copy(self)
+
+ copy = __copy__
+
+ def __deepcopy__(self):
+ """
+ Returns a deepcopy of the `Path`. The `Path` will not be
+ readonly, even if the source `Path` is.
+ """
+ return self.__class__(
+ self.vertices.copy(), self.codes.copy(),
+ _interpolation_steps=self._interpolation_steps)
+
+ deepcopy = __deepcopy__
@classmethod
def make_compound_path_from_polys(cls, XY):
@@ -420,7 +526,8 @@ def unit_rectangle(cls):
if cls._unit_rectangle is None:
cls._unit_rectangle = \
cls([[0.0, 0.0], [1.0, 0.0], [1.0, 1.0], [0.0, 1.0], [0.0, 0.0]],
- [cls.MOVETO, cls.LINETO, cls.LINETO, cls.LINETO, cls.CLOSEPOLY])
+ [cls.MOVETO, cls.LINETO, cls.LINETO, cls.LINETO, cls.CLOSEPOLY],
+ readonly=True)
return cls._unit_rectangle
_unit_regular_polygons = WeakValueDictionary()
@@ -447,7 +554,7 @@ def unit_regular_polygon(cls, numVertices):
codes[0] = cls.MOVETO
codes[1:-1] = cls.LINETO
codes[-1] = cls.CLOSEPOLY
- path = cls(verts, codes)
+ path = cls(verts, codes, readonly=True)
if numVertices <= 16:
cls._unit_regular_polygons[numVertices] = path
return path
@@ -478,7 +585,7 @@ def unit_regular_star(cls, numVertices, innerCircle=0.5):
codes[0] = cls.MOVETO
codes[1:-1] = cls.LINETO
codes[-1] = cls.CLOSEPOLY
- path = cls(verts, codes)
+ path = cls(verts, codes, readonly=True)
if numVertices <= 16:
cls._unit_regular_polygons[(numVertices, innerCircle)] = path
return path
@@ -552,7 +659,7 @@ def unit_circle(cls):
codes[0] = cls.MOVETO
codes[-1] = cls.CLOSEPOLY
- cls._unit_circle = cls(vertices, codes)
+ cls._unit_circle = cls(vertices, codes, readonly=True)
return cls._unit_circle
_unit_circle_righthalf = None
@@ -600,7 +707,7 @@ def unit_circle_righthalf(cls):
codes[0] = cls.MOVETO
codes[-1] = cls.CLOSEPOLY
- cls._unit_circle_righthalf = cls(vertices, codes)
+ cls._unit_circle_righthalf = cls(vertices, codes, readonly=True)
return cls._unit_circle_righthalf
@classmethod
@@ -679,7 +786,7 @@ def arc(cls, theta1, theta2, n=None, is_wedge=False):
vertices[vertex_offset+2:end:3, 0] = xB
vertices[vertex_offset+2:end:3, 1] = yB
- return cls(vertices, codes)
+ return cls(vertices, codes, readonly=True)
@classmethod
def wedge(cls, theta1, theta2, n=None):
View
3 lib/matplotlib/tests/test_bbox_tight.py
@@ -74,8 +74,7 @@ def test_bbox_inches_tight_clipping():
transform=ax.transData,
facecolor='blue', alpha=0.5)
- path = mpath.Path.unit_regular_star(5)
- path.vertices = path.vertices.copy()
+ path = mpath.Path.unit_regular_star(5).deepcopy()
path.vertices *= 0.25
patch.set_clip_path(path, transform=ax.transAxes)
plt.gcf().artists.append(patch)
View
6 lib/matplotlib/tests/test_patches.py
@@ -83,13 +83,11 @@ def test_clip_to_bbox():
ax.set_xlim([-18, 20])
ax.set_ylim([-150, 100])
- star = mpath.Path.unit_regular_star(8)
- path = mpath.Path(star.vertices.copy(), star.codes)
+ path = mpath.Path.unit_regular_star(8).deepcopy()
path.vertices *= [10, 100]
path.vertices -= [5, 25]
- circle = mpath.Path.unit_circle()
- path2 = mpath.Path(circle.vertices.copy(), circle.codes)
+ path2 = mpath.Path.unit_circle().deepcopy()
@pelson
pelson May 15, 2013

I think there is an example which does something similar too (http://matplotlib.org/examples/pylab_examples/marker_path.html).

@mdboom
mdboom May 15, 2013

Thanks. I'll update the example as well.

path2.vertices *= [10, 100]
path2.vertices += [10, -25]
View
10 lib/matplotlib/tests/test_path.py
@@ -0,0 +1,10 @@
+from matplotlib.path import Path
+from nose.tools import assert_raises
+
+def test_readonly_path():
+ path = Path.unit_circle()
+
+ def modify_vertices():
+ path.vertices = path.vertices * 2.0
+
+ assert_raises(AttributeError, modify_vertices)