Skip to content

Commit

Permalink
Rename InternalPoint3D to Point3D, Point3D to Point3DLike and…
Browse files Browse the repository at this point in the history
… other point-related type aliases (#4027)
  • Loading branch information
chopan050 authored Dec 17, 2024
1 parent 112c99b commit dbad8a8
Show file tree
Hide file tree
Showing 21 changed files with 655 additions and 368 deletions.
62 changes: 40 additions & 22 deletions docs/source/contributing/docs/types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,51 @@ in space. For example:

.. code-block:: python
def status2D(coord: Point2D) -> None:
def print_point2D(coord: Point2DLike) -> None:
x, y = coord
print(f"Point at {x=},{y=}")
def status3D(coord: Point3D) -> None:
def print_point3D(coord: Point3DLike) -> None:
x, y, z = coord
print(f"Point at {x=},{y=},{z=}")
def get_statuses(coords: Point2D_Array | Point3D_Array) -> None:
def print_point_array(coords: Point2DLike_Array | Point3DLike_Array) -> None:
for coord in coords:
if len(coord) == 2:
# it's a Point2D
status2D(coord)
# it's a Point2DLike
print_point2D(coord)
else:
# it's a point3D
status3D(coord)
It's important to realize that the status functions accepted both
tuples/lists of the correct length, and ``NDArray``'s of the correct shape.
If they only accepted ``NDArray``'s, we would use their ``Internal`` counterparts:
:class:`~.typing.InternalPoint2D`, :class:`~.typing.InternalPoint3D`, :class:`~.typing.InternalPoint2D_Array` and :class:`~.typing.InternalPoint3D_Array`.

In general, the type aliases prefixed with ``Internal`` should never be used on
user-facing classes and functions, but should be reserved for internal behavior.
# it's a Point3DLike
print_point3D(coord)
def shift_point_up(coord: Point3DLike) -> Point3D:
result = np.asarray(coord)
result += UP
print(f"New point: {result}")
return result
Notice that the last function, ``shift_point_up()``, accepts a
:class:`~.Point3DLike` as a parameter and returns a :class:`~.Point3D`. A
:class:`~.Point3D` always represents a NumPy array consisting of 3 floats,
whereas a :class:`~.Point3DLike` can represent anything resembling a 3D point:
either a NumPy array or a tuple/list of 3 floats, hence the ``Like`` word. The
same happens with :class:`~.Point2D`, :class:`~.Point2D_Array` and
:class:`~.Point3D_Array`, and their ``Like`` counterparts
:class:`~.Point2DLike`, :class:`~.Point2DLike_Array` and
:class:`~.Point3DLike_Array`.

The rule for typing functions is: **make parameter types as broad as possible,
and return types as specific as possible.** Therefore, for functions which are
intended to be called by users, **we should always, if possible, accept**
``Like`` **types as parameters and return NumPy, non-** ``Like`` **types.** The
main reason is to be more flexible with users who might want to pass tuples or
lists as arguments rather than NumPy arrays, because it's more convenient. The
last function, ``shift_point_up()``, is an example of it.

Internal functions which are *not* meant to be called by users may accept
non-``Like`` parameters if necessary.

Vectors
~~~~~~~
Expand All @@ -61,20 +80,19 @@ consider this slightly contrived function:
def shift_mobject(mob: M, direction: Vector3D, scale_factor: float = 1) -> M:
return mob.shift(direction * scale_factor)
Here we see an important example of the difference. ``direction`` can not, and
should not, be typed as a :class:`~.typing.Point3D` because the function does not accept tuples/lists,
like ``direction=(0, 1, 0)``. You could type it as :class:`~.typing.InternalPoint3D` and
the type checker and linter would be happy; however, this makes the code harder
to understand.
Here we see an important example of the difference. ``direction`` should not be
typed as a :class:`~.Point3D`, because it represents a direction along
which to shift a :class:`~.Mobject`, not a position in space.

As a general rule, if a parameter is called ``direction`` or ``axis``,
it should be type hinted as some form of :class:`~.VectorND`.

.. warning::

This is not always true. For example, as of Manim 0.18.0, the direction
parameter of the :class:`.Vector` Mobject should be ``Point2D | Point3D``,
as it can also accept ``tuple[float, float]`` and ``tuple[float, float, float]``.
parameter of the :class:`.Vector` Mobject should be
``Point2DLike | Point3DLike``, as it can also accept ``tuple[float, float]``
and ``tuple[float, float, float]``.

Colors
------
Expand Down
67 changes: 39 additions & 28 deletions manim/mobject/geometry/arc.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,9 @@ def construct(self):
from manim.mobject.text.tex_mobject import SingleStringMathTex, Tex
from manim.mobject.text.text_mobject import Text
from manim.typing import (
CubicBezierPoints,
InternalPoint3D,
Point3D,
QuadraticBezierPoints,
Point3DLike,
QuadraticSpline,
Vector3D,
)

Expand Down Expand Up @@ -269,22 +268,27 @@ def get_tip(self) -> VMobject:
def get_default_tip_length(self) -> float:
return self.tip_length

def get_first_handle(self) -> InternalPoint3D:
def get_first_handle(self) -> Point3D:
# Type inference of extracting an element from a list, is not
# supported by numpy, see this numpy issue
# https://github.com/numpy/numpy/issues/16544
return self.points[1]
first_handle: Point3D = self.points[1]
return first_handle

def get_last_handle(self) -> InternalPoint3D:
return self.points[-2]
def get_last_handle(self) -> Point3D:
# Type inference of extracting an element from a list, is not
# supported by numpy, see this numpy issue
# https://github.com/numpy/numpy/issues/16544
last_handle: Point3D = self.points[-2]
return last_handle

def get_end(self) -> InternalPoint3D:
def get_end(self) -> Point3D:
if self.has_tip():
return self.tip.get_start()
else:
return super().get_end()

def get_start(self) -> InternalPoint3D:
def get_start(self) -> Point3D:
if self.has_start_tip():
return self.start_tip.get_start()
else:
Expand Down Expand Up @@ -316,14 +320,14 @@ def __init__(
start_angle: float = 0,
angle: float = TAU / 4,
num_components: int = 9,
arc_center: InternalPoint3D = ORIGIN,
arc_center: Point3DLike = ORIGIN,
**kwargs: Any,
):
if radius is None: # apparently None is passed by ArcBetweenPoints
radius = 1.0
self.radius = radius
self.num_components = num_components
self.arc_center = arc_center
self.arc_center: Point3D = np.asarray(arc_center)
self.start_angle = start_angle
self.angle = angle
self._failed_to_get_center: bool = False
Expand Down Expand Up @@ -351,7 +355,7 @@ def init_points(self) -> None:
@staticmethod
def _create_quadratic_bezier_points(
angle: float, start_angle: float = 0, n_components: int = 8
) -> QuadraticBezierPoints:
) -> QuadraticSpline:
samples = np.array(
[
[np.cos(a), np.sin(a), 0]
Expand Down Expand Up @@ -394,7 +398,7 @@ def _set_pre_positioned_points(self) -> None:
handles2 = anchors[1:] - (d_theta / 3) * tangent_vectors[1:]
self.set_anchors_and_handles(anchors[:-1], handles1, handles2, anchors[1:])

def get_arc_center(self, warning: bool = True) -> InternalPoint3D:
def get_arc_center(self, warning: bool = True) -> Point3D:
"""Looks at the normals to the first two
anchors, and finds their intersection points
"""
Expand Down Expand Up @@ -422,7 +426,7 @@ def get_arc_center(self, warning: bool = True) -> InternalPoint3D:
self._failed_to_get_center = True
return np.array(ORIGIN)

def move_arc_center_to(self, point: InternalPoint3D) -> Self:
def move_arc_center_to(self, point: Point3DLike) -> Self:
self.shift(point - self.get_arc_center())
return self

Expand Down Expand Up @@ -454,8 +458,8 @@ def construct(self):

def __init__(
self,
start: Point3D,
end: Point3D,
start: Point3DLike,
end: Point3DLike,
angle: float = TAU / 4,
radius: float | None = None,
**kwargs: Any,
Expand Down Expand Up @@ -484,14 +488,18 @@ def __init__(
if radius is None:
center = self.get_arc_center(warning=False)
if not self._failed_to_get_center:
temp_radius: float = np.linalg.norm(np.array(start) - np.array(center))
self.radius = temp_radius
# np.linalg.norm returns floating[Any] which is not compatible with float
self.radius = cast(
float, np.linalg.norm(np.array(start) - np.array(center))
)
else:
self.radius = np.inf


class CurvedArrow(ArcBetweenPoints):
def __init__(self, start_point: Point3D, end_point: Point3D, **kwargs: Any) -> None:
def __init__(
self, start_point: Point3DLike, end_point: Point3DLike, **kwargs: Any
) -> None:
from manim.mobject.geometry.tips import ArrowTriangleFilledTip

tip_shape = kwargs.pop("tip_shape", ArrowTriangleFilledTip)
Expand All @@ -500,7 +508,9 @@ def __init__(self, start_point: Point3D, end_point: Point3D, **kwargs: Any) -> N


class CurvedDoubleArrow(CurvedArrow):
def __init__(self, start_point: Point3D, end_point: Point3D, **kwargs: Any) -> None:
def __init__(
self, start_point: Point3DLike, end_point: Point3DLike, **kwargs: Any
) -> None:
if "tip_shape_end" in kwargs:
kwargs["tip_shape"] = kwargs.pop("tip_shape_end")
from manim.mobject.geometry.tips import ArrowTriangleFilledTip
Expand Down Expand Up @@ -637,7 +647,7 @@ def construct(self):

@staticmethod
def from_three_points(
p1: Point3D, p2: Point3D, p3: Point3D, **kwargs: Any
p1: Point3DLike, p2: Point3DLike, p3: Point3DLike, **kwargs: Any
) -> Circle:
"""Returns a circle passing through the specified
three points.
Expand All @@ -661,7 +671,8 @@ def construct(self):
perpendicular_bisector([np.asarray(p1), np.asarray(p2)]),
perpendicular_bisector([np.asarray(p2), np.asarray(p3)]),
)
radius: float = np.linalg.norm(p1 - center)
# np.linalg.norm returns floating[Any] which is not compatible with float
radius = cast(float, np.linalg.norm(p1 - center))
return Circle(radius=radius, **kwargs).shift(center)


Expand Down Expand Up @@ -698,7 +709,7 @@ def construct(self):

def __init__(
self,
point: Point3D = ORIGIN,
point: Point3DLike = ORIGIN,
radius: float = DEFAULT_DOT_RADIUS,
stroke_width: float = 0,
fill_opacity: float = 1.0,
Expand Down Expand Up @@ -1006,10 +1017,10 @@ def construct(self):

def __init__(
self,
start_anchor: CubicBezierPoints,
start_handle: CubicBezierPoints,
end_handle: CubicBezierPoints,
end_anchor: CubicBezierPoints,
start_anchor: Point3DLike,
start_handle: Point3DLike,
end_handle: Point3DLike,
end_anchor: Point3DLike,
**kwargs: Any,
) -> None:
super().__init__(**kwargs)
Expand Down Expand Up @@ -1097,7 +1108,7 @@ def construct(self):

def __init__(
self,
*vertices: Point3D,
*vertices: Point3DLike,
angle: float = PI / 4,
radius: float | None = None,
arc_config: list[dict] | None = None,
Expand Down
10 changes: 5 additions & 5 deletions manim/mobject/geometry/boolean_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
if TYPE_CHECKING:
from typing import Any

from manim.typing import InternalPoint3D_Array, Point2D_Array
from manim.typing import Point2DLike_Array, Point3D_Array, Point3DLike_Array

from ...constants import RendererType

Expand All @@ -30,17 +30,17 @@ class _BooleanOps(VMobject, metaclass=ConvertToOpenGL):

def _convert_2d_to_3d_array(
self,
points: Point2D_Array,
points: Point2DLike_Array | Point3DLike_Array,
z_dim: float = 0.0,
) -> InternalPoint3D_Array:
) -> Point3D_Array:
"""Converts an iterable with coordinates in 2D to 3D by adding
:attr:`z_dim` as the Z coordinate.
Parameters
----------
points:
points
An iterable of points.
z_dim:
z_dim
Default value for the Z coordinate.
Returns
Expand Down
4 changes: 2 additions & 2 deletions manim/mobject/geometry/labeled.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
if TYPE_CHECKING:
from typing import Any

from manim.typing import Point3D_Array
from manim.typing import Point3DLike_Array


class Label(VGroup):
Expand Down Expand Up @@ -344,7 +344,7 @@ def construct(self):

def __init__(
self,
*vertex_groups: Point3D_Array,
*vertex_groups: Point3DLike_Array,
label: str | Tex | MathTex | Text,
precision: float = 0.01,
label_config: dict[str, Any] | None = None,
Expand Down
Loading

0 comments on commit dbad8a8

Please sign in to comment.