Skip to content

Refactor for issue 28444 [Introduce _Bbox3D to represent 3D axes limits] #30115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
31 changes: 31 additions & 0 deletions lib/matplotlib/transforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,37 @@
self.update_from_path(path, ignore=ignore,
updatex=updatex, updatey=updatey)

def update_from_bbox(self, bbox, ignore=False, updatex=True, updatey=True):
"""
Update this Bbox to include the bounds of another Bbox.

This method expands the current Bbox in-place to include the given *bbox*,
unless *ignore* is True, in which case the current Bbox is replaced with
the bounds of *bbox*. If the input *bbox* is already fully contained,
the current Bbox remains unchanged.

This is equivalent to performing an in-place union of the two Bboxes,
unless *ignore=True*, which resets the bounds.

Parameters
----------
bbox : Bbox
The Bbox to merge into this one.
ignore : bool, default: False
If True, the current bounds are ignored and set to match *bbox*.
If False, the current bounds are expanded to include *bbox*.
updatex : bool, default: True
Whether to update the x-dimension bounds.
updatey : bool, default: True
Whether to update the y-dimension bounds.
"""
if not updatex and not updatey:
return

Check warning on line 991 in lib/matplotlib/transforms.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/transforms.py#L991

Added line #L991 was not covered by tests

vertices = np.array([[bbox.x0, bbox.y0],[bbox.x1, bbox.y1]])
path = Path(vertices)
self.update_from_path(path, ignore=ignore, updatex=updatex, updatey=updatey)

@BboxBase.x0.setter
def x0(self, val):
self._points[0, 0] = val
Expand Down
1 change: 1 addition & 0 deletions lib/matplotlib/transforms.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class Bbox(BboxBase):
updatex: bool = ...,
updatey: bool = ...,
) -> None: ...
def update_from_bbox(self, bbox: Bbox, ignore: bool = ..., updatex: bool = ..., updatey: bool = ...) -> None: ...
@property
def minpos(self) -> float: ...
@property
Expand Down
39 changes: 39 additions & 0 deletions lib/mpl_toolkits/mplot3d/art3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
Collection, LineCollection, PolyCollection, PatchCollection, PathCollection)
from matplotlib.patches import Patch
from . import proj3d
from .bbox3d import _Bbox3d


def _norm_angle(a):
Expand Down Expand Up @@ -97,6 +98,19 @@
return mask


def create_bbox3d_from_array(arr):
arr = np.asarray(arr)
if arr.ndim != 2 or arr.shape[1] != 3:
raise ValueError("Expected array of shape (N, 3)")

Check warning on line 104 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L104

Added line #L104 was not covered by tests
if arr.shape[0] == 0:
return _Bbox3d(((np.inf, -np.inf), (np.inf, -np.inf), (np.inf, -np.inf)))

Check warning on line 106 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L106

Added line #L106 was not covered by tests
mins = np.nanmin(arr, axis=0)
maxs = np.nanmax(arr, axis=0)
xmin, ymin, zmin = mins
xmax, ymax, zmax = maxs
return _Bbox3d(((xmin, xmax), (ymin, ymax), (zmin, zmax)))


class Text3D(mtext.Text):
"""
Text object with 3D position and direction.
Expand Down Expand Up @@ -331,6 +345,10 @@
super().draw(renderer)
self.stale = False

def _get_datalim3d(self):
xs, ys, zs = self._verts3d
return create_bbox3d_from_array(np.column_stack((xs, ys, zs)))

Check warning on line 350 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L349-L350

Added lines #L349 - L350 were not covered by tests


def line_2d_to_3d(line, zs=0, zdir='z', axlim_clip=False):
"""
Expand Down Expand Up @@ -513,6 +531,10 @@
minz = np.nan
return minz

def _get_datalim3d(self):
segments = np.concatenate(self._segments3d)
return create_bbox3d_from_array(segments)


def line_collection_2d_to_3d(col, zs=0, zdir='z', axlim_clip=False):
"""Convert a `.LineCollection` to a `.Line3DCollection` object."""
Expand Down Expand Up @@ -591,6 +613,9 @@
self._path2d = mpath.Path(np.ma.column_stack([vxs, vys]))
return min(vzs)

def _get_datalim3d(self):
return create_bbox3d_from_array(self._segment3d)

Check warning on line 617 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L617

Added line #L617 was not covered by tests


class PathPatch3D(Patch3D):
"""
Expand Down Expand Up @@ -653,6 +678,9 @@
self._path2d = mpath.Path(np.ma.column_stack([vxs, vys]), self._code3d)
return min(vzs)

def _get_datalim3d(self):
return create_bbox3d_from_array(self._segment3d)

Check warning on line 682 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L682

Added line #L682 was not covered by tests


def _get_patch_verts(patch):
"""Return a list of vertices for the path of a patch."""
Expand Down Expand Up @@ -832,6 +860,10 @@
return self.get_facecolor()
return self._maybe_depth_shade_and_sort_colors(super().get_edgecolor())

def _get_datalim3d(self):
xs, ys, zs = self._offsets3d
return create_bbox3d_from_array(np.column_stack((xs, ys, zs)))


def _get_data_scale(X, Y, Z):
"""
Expand Down Expand Up @@ -1087,6 +1119,10 @@
return self.get_facecolor()
return self._maybe_depth_shade_and_sort_colors(super().get_edgecolor())

def _get_datalim3d(self):
xs, ys, zs = self._offsets3d
return create_bbox3d_from_array(np.column_stack((xs, ys, zs)))

Check warning on line 1124 in lib/mpl_toolkits/mplot3d/art3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/art3d.py#L1123-L1124

Added lines #L1123 - L1124 were not covered by tests


def patch_collection_2d_to_3d(
col,
Expand Down Expand Up @@ -1464,6 +1500,9 @@
self.do_3d_projection()
return np.asarray(self._edgecolors2d)

def _get_datalim3d(self):
return create_bbox3d_from_array(self._faces.reshape(-1, 3))


def poly_collection_2d_to_3d(col, zs=0, zdir='z', axlim_clip=False):
"""
Expand Down
41 changes: 29 additions & 12 deletions lib/mpl_toolkits/mplot3d/axes3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,25 @@
# Let autoscale_view figure out how to use this data.
self.autoscale_view()

def auto_scale_lim(self, bbox3d, had_data=False):
"""
Expand the 3D axes data limits to include the given Bbox3d.

Parameters
----------
bbox3d : _Bbox3d
The 3D bounding box to incorporate into the data limits.
had_data : bool, default: False
Whether the axes already had data limits set before.
"""
self.xy_dataLim.update_from_bbox(bbox3d.to_bbox_xy(), ignore=not had_data)
self.zz_dataLim.update_from_bbox(bbox3d.to_bbox_zz(), ignore=not had_data)
if not had_data:
self._xy_dataLim_set = True
self._zz_dataLim_set = True
self.autoscale_view()


def autoscale_view(self, tight=None,
scalex=True, scaley=True, scalez=True):
"""
Expand Down Expand Up @@ -2888,18 +2907,16 @@
col.set_sort_zpos(zsortval)

if autolim:
if isinstance(col, art3d.Line3DCollection):
self.auto_scale_xyz(*np.array(col._segments3d).transpose(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything use auto_scale_xyz internally after this? Should still keep it since it's a public method, but should make sure everything is switched over

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see in your initial comment that this is meant to lay the groundwork for switching everything over in a follow-on PR - I think that incrementalism is fine but also feel free to put everything into one if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Scott! I did a full review of the remaining auto_scale_xyz() usages in axes3d.py. I noticed that:

1 - Some cases are tied to 3D collections like Line3DCollection and Poly3DCollection, which now have _get_datalim3d() implemented. These can be cleanly refactored to use auto_scale_lim() instead.

2 - Other cases operate directly on raw X, Y, Z array inputs. In these, there's no associated artist or _Bbox3d abstraction, so converting to auto_scale_lim() would require constructing temporary bounding boxes manually — which feels like a larger shift in design.

Before I proceed, I wanted to check:
Would you prefer that I refactor only the collection-based uses in this PR, and leave the raw-data ones as they are? Or should I aim for a broader unification?

Thanks in advance for the guidance!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think it makes sense to move to bounding boxes for the cases where they're currently operating on raw X, Y, Z array inputs. It will make clearer what's going on, and since the array data is not being copied should not introduce any overhead.

had_data=had_data)
elif isinstance(col, art3d.Poly3DCollection):
self.auto_scale_xyz(col._faces[..., 0],
col._faces[..., 1],
col._faces[..., 2], had_data=had_data)
elif isinstance(col, art3d.Patch3DCollection):
pass
# FIXME: Implement auto-scaling function for Patch3DCollection
# Currently unable to do so due to issues with Patch3DCollection
# See https://github.com/matplotlib/matplotlib/issues/14298 for details
if hasattr(col, "_get_datalim3d"):
bbox3d = col._get_datalim3d()
self.auto_scale_lim(bbox3d, had_data=had_data)
else:
warnings.warn(

Check warning on line 2914 in lib/mpl_toolkits/mplot3d/axes3d.py

View check run for this annotation

Codecov / codecov/patch

lib/mpl_toolkits/mplot3d/axes3d.py#L2914

Added line #L2914 was not covered by tests
(f"{type(col).__name__} does not implement `_get_datalim3d`,"
" so it will not be autoscaled."),
category=UserWarning,
stacklevel=2
)

collection = super().add_collection(col)
return collection
Expand Down
39 changes: 39 additions & 0 deletions lib/mpl_toolkits/mplot3d/bbox3d.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from matplotlib.transforms import Bbox


class _Bbox3d:
"""
A helper class to represent a 3D bounding box.

This class stores the minimum and maximum extents of data in 3D space
(xmin, xmax, ymin, ymax, zmin, zmax). It provides methods to convert
these extents into 2D bounding boxes (`Bbox`) for compatibility with
existing matplotlib functionality.

Attributes
----------
xmin, xmax : float
The minimum and maximum extents along the x-axis.
ymin, ymax : float
The minimum and maximum extents along the y-axis.
zmin, zmax : float
The minimum and maximum extents along the z-axis.

Methods
-------
to_bbox_xy():
Converts the x and y extents into a 2D `Bbox`.
to_bbox_zz():
Converts the z extents into a 2D `Bbox`, with the y-component unused.
"""
def __init__(self, points):
((self.xmin, self.xmax),
(self.ymin, self.ymax),
(self.zmin, self.zmax)) = points

def to_bbox_xy(self):
return Bbox(((self.xmin, self.ymin), (self.xmax, self.ymax)))

def to_bbox_zz(self):
# first component contains z, second is unused
return Bbox(((self.zmin, 0), (self.zmax, 0)))
1 change: 1 addition & 0 deletions lib/mpl_toolkits/mplot3d/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ python_sources = [
'axes3d.py',
'axis3d.py',
'proj3d.py',
'bbox3d.py'
]

py3.install_sources(python_sources, subdir: 'mpl_toolkits/mplot3d')
Expand Down
Loading