From 33ac00b88170780ec1dc09cd29fb57cfd703893a Mon Sep 17 00:00:00 2001 From: Guillaume Vauvert Date: Thu, 2 Jan 2025 16:54:24 +0000 Subject: [PATCH 1/5] Issue-4087 Add bug test --- tests/module/mobject/mobject/test_mobject.py | 26 ++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/module/mobject/mobject/test_mobject.py b/tests/module/mobject/mobject/test_mobject.py index 89deea93c9..6fbf64c764 100644 --- a/tests/module/mobject/mobject/test_mobject.py +++ b/tests/module/mobject/mobject/test_mobject.py @@ -149,8 +149,8 @@ def test_mobject_dimensions_mobjects_with_no_points_are_at_origin(): # group because now the bottom left corner is at [-5, -6.5, 0] # but the upper right corner is [0, 0, 0] instead of [-3, -3.5, 0] outer_group.add(VGroup()) - assert outer_group.width == 5 - assert outer_group.height == 6.5 + assert outer_group.width == 2 + assert outer_group.height == 3 def test_mobject_dimensions_has_points_and_children(): @@ -168,3 +168,25 @@ def test_mobject_dimensions_has_points_and_children(): assert inner_rect.width == 2 assert inner_rect.height == 1 assert inner_rect.depth == 0 + + +def test_mobject_width_height(): + for flow_order in ["ur", "dl"]: + for is_reversed in [False, True]: + # create a vgroup with some sub-elements + vgroup = VGroup() + sub_vgroup = VGroup() + square = Square(side_length=2.0) + sub_elements = [sub_vgroup, square] + if is_reversed: + sub_elements.reverse() + vgroup.add(sub_elements) + # arrange them to move sub-objects all above or below zero + direction = UR if flow_order == "ur" else DL + vgroup.shift(direction * 10) + vgroup.arrange_in_grid(rows=2, cols=2, flow_order=flow_order) + # These measures should be 2. + assert vgroup.length_over_dim(0) == pytest.approx(2.0) + assert vgroup.length_over_dim(1) == pytest.approx(2.0) + assert vgroup.height == pytest.approx(2.0) + assert vgroup.width == pytest.approx(2.0) From e9443ae37acefd655597b42ef4c2356803a9b2d2 Mon Sep 17 00:00:00 2001 From: Guillaume Vauvert Date: Thu, 2 Jan 2025 16:55:56 +0000 Subject: [PATCH 2/5] Issue-4087 Fix bug --- manim/mobject/mobject.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index 8557490ed9..c754407856 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -1985,7 +1985,7 @@ def reduce_across_dimension(self, reduce_func: Callable, dim: int): assert dim <= 2 if len(self.submobjects) == 0 and len(self.points) == 0: # If we have no points and no submobjects, return 0 (e.g. center) - return 0 + return None # If we do not have points (but do have submobjects) # use only the points from those. @@ -1998,7 +1998,11 @@ def reduce_across_dimension(self, reduce_func: Callable, dim: int): # smallest dimension they have and compare it to the return value. for mobj in self.submobjects: value = mobj.reduce_across_dimension(reduce_func, dim) - rv = value if rv is None else reduce_func([value, rv]) + if rv is None: + rv = value + else: + if value is not None: + rv = reduce_func([value, rv]) return rv def nonempty_submobjects(self) -> list[Self]: @@ -2149,10 +2153,11 @@ def get_nadir(self) -> Point3D: def length_over_dim(self, dim: int) -> float: """Measure the length of an :class:`~.Mobject` in a certain direction.""" - return self.reduce_across_dimension( - max, - dim, - ) - self.reduce_across_dimension(min, dim) + val_max = self.reduce_across_dimension(max, dim) + val_min = self.reduce_across_dimension(min, dim) + if val_max is None and val_min is None: + return 0 + return val_max - val_min def get_coord(self, dim: int, direction: Vector3D = ORIGIN): """Meant to generalize ``get_x``, ``get_y`` and ``get_z``""" From 1e59773e2a82cea58163c5cf84ebb3896445e6b1 Mon Sep 17 00:00:00 2001 From: Guillaume Vauvert Date: Thu, 2 Jan 2025 20:36:16 +0000 Subject: [PATCH 3/5] Issue-4087 Fix comment and add type hint --- manim/mobject/mobject.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py index c754407856..b0991430a6 100644 --- a/manim/mobject/mobject.py +++ b/manim/mobject/mobject.py @@ -1979,12 +1979,12 @@ def restore(self) -> Self: self.become(self.saved_state) return self - def reduce_across_dimension(self, reduce_func: Callable, dim: int): + def reduce_across_dimension(self, reduce_func: Callable, dim: int) -> float | None: """Find the min or max value from a dimension across all points in this and submobjects.""" assert dim >= 0 assert dim <= 2 if len(self.submobjects) == 0 and len(self.points) == 0: - # If we have no points and no submobjects, return 0 (e.g. center) + # If we have no points and no submobjects, return None return None # If we do not have points (but do have submobjects) From 733750166eb64604f52a995d725c31b890e32492 Mon Sep 17 00:00:00 2001 From: Guillaume Vauvert Date: Fri, 3 Jan 2025 08:23:51 +0000 Subject: [PATCH 4/5] Issue-4087 Fix comment --- tests/module/mobject/mobject/test_mobject.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/module/mobject/mobject/test_mobject.py b/tests/module/mobject/mobject/test_mobject.py index 6fbf64c764..8b8cf42a0f 100644 --- a/tests/module/mobject/mobject/test_mobject.py +++ b/tests/module/mobject/mobject/test_mobject.py @@ -144,10 +144,7 @@ def test_mobject_dimensions_mobjects_with_no_points_are_at_origin(): assert outer_group.width == 2 assert outer_group.height == 3 - # Adding a mobject with no points has a quirk of adding a "point" - # to [0, 0, 0] (the origin). This changes the size of the outer - # group because now the bottom left corner is at [-5, -6.5, 0] - # but the upper right corner is [0, 0, 0] instead of [-3, -3.5, 0] + # Adding a mobject with no points does not change its size outer_group.add(VGroup()) assert outer_group.width == 2 assert outer_group.height == 3 From 8b5d8313175fa95fe7e8f15470d072db587cdf5c Mon Sep 17 00:00:00 2001 From: Guillaume Vauvert Date: Fri, 3 Jan 2025 08:50:19 +0000 Subject: [PATCH 5/5] Issue-4087 Improve previous test instead of adding a new one --- tests/module/mobject/mobject/test_mobject.py | 45 ++++++-------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/tests/module/mobject/mobject/test_mobject.py b/tests/module/mobject/mobject/test_mobject.py index 8b8cf42a0f..b00b4133c5 100644 --- a/tests/module/mobject/mobject/test_mobject.py +++ b/tests/module/mobject/mobject/test_mobject.py @@ -3,7 +3,7 @@ import numpy as np import pytest -from manim import DL, UR, Circle, Mobject, Rectangle, Square, VGroup +from manim import DL, DR, UL, UR, Circle, Mobject, Rectangle, Square, VGroup def test_mobject_add(): @@ -136,18 +136,19 @@ def test_mobject_dimensions_nested_mobjects(): def test_mobject_dimensions_mobjects_with_no_points_are_at_origin(): - rect = Rectangle(width=2, height=3) - rect.move_to([-4, -5, 0]) - outer_group = VGroup(rect) + for direction in [DL, DR, UL, UR]: + rect = Rectangle(width=2, height=3) + rect.move_to(direction * 10) + outer_group = VGroup(rect) - # This is as one would expect - assert outer_group.width == 2 - assert outer_group.height == 3 + # This is as one would expect + assert outer_group.width == 2 + assert outer_group.height == 3 - # Adding a mobject with no points does not change its size - outer_group.add(VGroup()) - assert outer_group.width == 2 - assert outer_group.height == 3 + # Adding a mobject with no points does not change its size + outer_group.add(VGroup()) + assert outer_group.width == 2 + assert outer_group.height == 3 def test_mobject_dimensions_has_points_and_children(): @@ -165,25 +166,3 @@ def test_mobject_dimensions_has_points_and_children(): assert inner_rect.width == 2 assert inner_rect.height == 1 assert inner_rect.depth == 0 - - -def test_mobject_width_height(): - for flow_order in ["ur", "dl"]: - for is_reversed in [False, True]: - # create a vgroup with some sub-elements - vgroup = VGroup() - sub_vgroup = VGroup() - square = Square(side_length=2.0) - sub_elements = [sub_vgroup, square] - if is_reversed: - sub_elements.reverse() - vgroup.add(sub_elements) - # arrange them to move sub-objects all above or below zero - direction = UR if flow_order == "ur" else DL - vgroup.shift(direction * 10) - vgroup.arrange_in_grid(rows=2, cols=2, flow_order=flow_order) - # These measures should be 2. - assert vgroup.length_over_dim(0) == pytest.approx(2.0) - assert vgroup.length_over_dim(1) == pytest.approx(2.0) - assert vgroup.height == pytest.approx(2.0) - assert vgroup.width == pytest.approx(2.0)