From de4a94cadf813353413d4bd3dbd9d7e7121b176b Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Fri, 29 Apr 2022 21:51:51 -0600 Subject: [PATCH 1/2] FIX: Handle no-offsets in collection datalim This adds a check for whether the collection has offsets passed in initially. This is necessary to handle the (0, 0) offset case that is desired and distinguishing that from the initial empty offset of (0, 0). In particular, this is necessary for the non-transData IdentityTransform passed in during tight_layout calculations. --- lib/matplotlib/collections.py | 30 +++++++++++++----------- lib/matplotlib/tests/test_collections.py | 12 ++++++++++ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/matplotlib/collections.py b/lib/matplotlib/collections.py index 0aa37376a477..350f580702fe 100644 --- a/lib/matplotlib/collections.py +++ b/lib/matplotlib/collections.py @@ -195,8 +195,9 @@ def __init__(self, # default to zeros self._offsets = np.zeros((1, 2)) + self._has_offsets = offsets is not None - if offsets is not None: + if self._has_offsets: offsets = np.asanyarray(offsets, float) # Broadcast (2,) -> (1, 2) but nothing else. if offsets.shape == (2,): @@ -278,7 +279,7 @@ def get_datalim(self, transData): offsets = offsets.filled(np.nan) # get_path_collection_extents handles nan but not masked arrays - if len(paths) and len(offsets): + if len(paths): if any(transform.contains_branch_seperately(transData)): # collections that are just in data units (like quiver) # can properly have the axes limits set by their shape + @@ -290,18 +291,19 @@ def get_datalim(self, transData): offset_trf.transform_non_affine(offsets), offset_trf.get_affine().frozen()) - # this is for collections that have their paths (shapes) - # in physical, axes-relative, or figure-relative units - # (i.e. like scatter). We can't uniquely set limits based on - # those shapes, so we just set the limits based on their - # location. - offsets = (offset_trf - transData).transform(offsets) - # note A-B means A B^{-1} - offsets = np.ma.masked_invalid(offsets) - if not offsets.mask.all(): - bbox = transforms.Bbox.null() - bbox.update_from_data_xy(offsets) - return bbox + if self._has_offsets: + # this is for collections that have their paths (shapes) + # in physical, axes-relative, or figure-relative units + # (i.e. like scatter). We can't uniquely set limits based on + # those shapes, so we just set the limits based on their + # location. + offsets = (offset_trf - transData).transform(offsets) + # note A-B means A B^{-1} + offsets = np.ma.masked_invalid(offsets) + if not offsets.mask.all(): + bbox = transforms.Bbox.null() + bbox.update_from_data_xy(offsets) + return bbox return transforms.Bbox.null() def get_window_extent(self, renderer): diff --git a/lib/matplotlib/tests/test_collections.py b/lib/matplotlib/tests/test_collections.py index 2dffdf9cf893..e4b0c234e4b5 100644 --- a/lib/matplotlib/tests/test_collections.py +++ b/lib/matplotlib/tests/test_collections.py @@ -9,6 +9,7 @@ import matplotlib.pyplot as plt import matplotlib.collections as mcollections import matplotlib.colors as mcolors +import matplotlib.path as mpath import matplotlib.transforms as mtransforms from matplotlib.collections import (Collection, LineCollection, EventCollection, PolyCollection) @@ -291,6 +292,17 @@ def test_null_collection_datalim(): mtransforms.Bbox.null().get_points()) +def test_no_offsets_datalim(): + # A collection with no offsets and a non transData + # transform should return a null bbox + ax = plt.axes() + coll = mcollections.PathCollection([mpath.Path([(0, 0), (1, 0)])]) + ax.add_collection(coll) + coll_data_lim = coll.get_datalim(mtransforms.IdentityTransform()) + assert_array_equal(coll_data_lim.get_points(), + mtransforms.Bbox.null().get_points()) + + def test_add_collection(): # Test if data limits are unchanged by adding an empty collection. # GitHub issue #1490, pull #1497. From e95384083dd2319a1b720789244c68c8d1c516bb Mon Sep 17 00:00:00 2001 From: Greg Lucas Date: Sat, 30 Apr 2022 08:12:37 -0600 Subject: [PATCH 2/2] MNT: Use get_offsets() to handle the default/None case This avoids carrying around an extra offsetsNone/has_offsets variable to keep track of the default case. Instead calling get_offsets() to return the default zeros case, and then internally checking the None/default case in get_datalim() which is the only place where this information is needed. --- lib/matplotlib/collections.py | 92 +++++++++++++++++------------------ 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/lib/matplotlib/collections.py b/lib/matplotlib/collections.py index 350f580702fe..2e1d75c6d411 100644 --- a/lib/matplotlib/collections.py +++ b/lib/matplotlib/collections.py @@ -62,7 +62,6 @@ class Collection(artist.Artist, cm.ScalarMappable): mappable will be used to set the ``facecolors`` and ``edgecolors``, ignoring those that were manually passed in. """ - _offsets = np.zeros((0, 2)) #: Either a list of 3x3 arrays or an Nx3x3 array (representing N #: transforms), suitable for the `all_transforms` argument to #: `~matplotlib.backend_bases.RendererBase.draw_path_collection`; @@ -193,16 +192,12 @@ def __init__(self, else: self._joinstyle = None - # default to zeros - self._offsets = np.zeros((1, 2)) - self._has_offsets = offsets is not None - - if self._has_offsets: + if offsets is not None: offsets = np.asanyarray(offsets, float) # Broadcast (2,) -> (1, 2) but nothing else. if offsets.shape == (2,): offsets = offsets[None, :] - self._offsets = offsets + self._offsets = offsets self._offset_transform = offset_transform @@ -264,9 +259,12 @@ def get_datalim(self, transData): # if the offsets are in some coords other than data, # then don't use them for autoscaling. return transforms.Bbox.null() - offsets = self._offsets + offsets = self.get_offsets() paths = self.get_paths() + if not len(paths): + # No paths to transform + return transforms.Bbox.null() if not transform.is_affine: paths = [transform.transform_path_non_affine(p) for p in paths] @@ -275,35 +273,34 @@ def get_datalim(self, transData): # transforms.get_affine().contains_branch(transData). But later, # be careful to only apply the affine part that remains. - if isinstance(offsets, np.ma.MaskedArray): - offsets = offsets.filled(np.nan) - # get_path_collection_extents handles nan but not masked arrays - - if len(paths): - if any(transform.contains_branch_seperately(transData)): - # collections that are just in data units (like quiver) - # can properly have the axes limits set by their shape + - # offset. LineCollections that have no offsets can - # also use this algorithm (like streamplot). - return mpath.get_path_collection_extents( - transform.get_affine() - transData, paths, - self.get_transforms(), - offset_trf.transform_non_affine(offsets), - offset_trf.get_affine().frozen()) - - if self._has_offsets: - # this is for collections that have their paths (shapes) - # in physical, axes-relative, or figure-relative units - # (i.e. like scatter). We can't uniquely set limits based on - # those shapes, so we just set the limits based on their - # location. - offsets = (offset_trf - transData).transform(offsets) - # note A-B means A B^{-1} - offsets = np.ma.masked_invalid(offsets) - if not offsets.mask.all(): - bbox = transforms.Bbox.null() - bbox.update_from_data_xy(offsets) - return bbox + if any(transform.contains_branch_seperately(transData)): + # collections that are just in data units (like quiver) + # can properly have the axes limits set by their shape + + # offset. LineCollections that have no offsets can + # also use this algorithm (like streamplot). + if isinstance(offsets, np.ma.MaskedArray): + offsets = offsets.filled(np.nan) + # get_path_collection_extents handles nan but not masked arrays + return mpath.get_path_collection_extents( + transform.get_affine() - transData, paths, + self.get_transforms(), + offset_trf.transform_non_affine(offsets), + offset_trf.get_affine().frozen()) + + # NOTE: None is the default case where no offsets were passed in + if self._offsets is not None: + # this is for collections that have their paths (shapes) + # in physical, axes-relative, or figure-relative units + # (i.e. like scatter). We can't uniquely set limits based on + # those shapes, so we just set the limits based on their + # location. + offsets = (offset_trf - transData).transform(offsets) + # note A-B means A B^{-1} + offsets = np.ma.masked_invalid(offsets) + if not offsets.mask.all(): + bbox = transforms.Bbox.null() + bbox.update_from_data_xy(offsets) + return bbox return transforms.Bbox.null() def get_window_extent(self, renderer): @@ -316,7 +313,7 @@ def _prepare_points(self): transform = self.get_transform() offset_trf = self.get_offset_transform() - offsets = self._offsets + offsets = self.get_offsets() paths = self.get_paths() if self.have_units(): @@ -327,10 +324,9 @@ def _prepare_points(self): xs = self.convert_xunits(xs) ys = self.convert_yunits(ys) paths.append(mpath.Path(np.column_stack([xs, ys]), path.codes)) - if offsets.size: - xs = self.convert_xunits(offsets[:, 0]) - ys = self.convert_yunits(offsets[:, 1]) - offsets = np.column_stack([xs, ys]) + xs = self.convert_xunits(offsets[:, 0]) + ys = self.convert_yunits(offsets[:, 1]) + offsets = np.column_stack([xs, ys]) if not transform.is_affine: paths = [transform.transform_path_non_affine(path) @@ -559,7 +555,8 @@ def set_offsets(self, offsets): def get_offsets(self): """Return the offsets for the collection.""" - return self._offsets + # Default to zeros in the no-offset (None) case + return np.zeros((1, 2)) if self._offsets is None else self._offsets def _get_default_linewidth(self): # This may be overridden in a subclass. @@ -2156,13 +2153,12 @@ def draw(self, renderer): renderer.open_group(self.__class__.__name__, self.get_gid()) transform = self.get_transform() offset_trf = self.get_offset_transform() - offsets = self._offsets + offsets = self.get_offsets() if self.have_units(): - if len(self._offsets): - xs = self.convert_xunits(self._offsets[:, 0]) - ys = self.convert_yunits(self._offsets[:, 1]) - offsets = np.column_stack([xs, ys]) + xs = self.convert_xunits(offsets[:, 0]) + ys = self.convert_yunits(offsets[:, 1]) + offsets = np.column_stack([xs, ys]) self.update_scalarmappable()