Repository: ManimCommunity/manim
Pull request: Added documentation and testing for ``path_func`` keyword argument of :class:`.Transform`
<!-- Thank you for contributing to Manim! Learn more about the process in our contributing guidelines: https://docs.manim.community/en/latest/contributing.html -->

## Overview: What does this pull request change?
The Transform class's handling of path_func parameters is unintuitively implemented and undocumented, which was highlighted by Issue #2724. This PR:
- Adds three tests to test_transform to verify the handling of these parameters
- Refactors the relevant constructor to make the behavior more clear
- Adds documentation of Transform's parameters, especially with regard to `path_func` and `path_arc`
- Resolves #2724 

<!--changelog-start-->

<!--changelog-end-->

## Motivation and Explanation: Why and how do your changes improve the library?

## Links to added or changed documentation pages
<!-- Please add links to the affected documentation pages (edit the description after opening the PR). The link to the documentation for your PR is https://manimce--####.org.readthedocs.build/en/####/, where #### represents the PR number. -->
- [manim.animation.transform.Transform](https://manimce--2771.org.readthedocs.build/en/2771/reference/manim.animation.transform.Transform.html)

## Further Information and Comments
No library behavior was changed.

<!-- Thank you again for contributing! Do not modify the lines below, they are for reviewers. -->
## Reviewer Checklist
- [x] The PR title is descriptive enough for the changelog, and the PR is labeled correctly
- [x] If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
- [x] If applicable: newly added functions and classes are tested


Problem statement:
Transform: missing documentation of path_arc_centers keyword argument
## Description of bug / unexpected behavior
<!-- Add a clear and concise description of the problem you encountered. -->
The `Transform` constructor accepts a keyword argument, `path_arc_centers`, which is only being used once and this use is not working as intended.

Inspecting `Transform`'s `__init__` code, at line 64, there's the following code:

```python
if self.path_arc_centers is not None:
    self.path_func = path_along_circles(
        path_arc,
        self.path_arc_centers,
        self.path_arc_axis,
    )

self.path_func: Callable | None = path_func
```

As it can be seen, if `self.path_arc_centers` is not None, it is used to construct a path function which is assigned to `self.path_func`. But inmediately after this, `self.path_func` is assigned the `path_func` keyword argument in all cases, overwriting any possibly stored value in `self.path_func`.

Thus, this `path_arc_centers` keyword argument is not working as intended, and as mentioned before, it's not being used anywhere else inside `Transform`'s definition.

## Expected behavior
<!-- Add a clear and concise description of what you expected to happen. -->

## How to reproduce the issue
<!-- Provide a piece of code illustrating the undesired behavior. -->

## Additional media files

## Logs

## System specifications

## Additional comments
<!-- Add further context that you think might be relevant for this issue here. -->

Hints from issue discussion:
Contrary to what I thought before (and also contrary to any sort of reasonable coding practice), this is actually wrong. When we discussed this earlier, I missed that `path_func` is actually a property whose setter checks whether the value that is being assigned is `None` (and if so, the actual function is not changed).

So actually, if `path_arc_centers` is passed, a "proper" `path_func` is constructed and assigned (which the setter also does properly, as not-`None` is passed), then `self.path_func = None` is run and the setter actually *does not* change the value of the internal attribute.

Which means that this is not a bug, but just something that needs to be documented better (and I actually also think that changing the two blocks would not hurt either). I actually played around with the `path_arc_centers` argument briefly, but I'm still not entirely sure how exactly it should be used.

Commit message: Add Transform test for conflicting path settings

Patch to review:
diff --git a/manim/animation/transform.py b/manim/animation/transform.py
index 624d36f004..60159ecca9 100644
--- a/manim/animation/transform.py
+++ b/manim/animation/transform.py
@@ -46,6 +46,35 @@
 
 
 class Transform(Animation):
+    """A Transform transforms a Mobject into a target Mobject.
+
+    Parameters
+    ----------
+    mobject
+        The Mobject to be transformed. The Mobject will be mutated into the target.
+    target_mobject
+        The Mobject that will be transformed into. Will become invisible and replaced by mobject.
+    path_func
+        A function defining the path that the mobject will take to target_mobject,
+        see `manim.utils.paths`.
+    path_arc
+        The arc (in radians) that the mobject will take to reach target if using a circular path
+        arc, see `path_arc_centers`.
+    path_arc_axis
+        The axis to rotate along if using a circular path arc, see `path_arc_centers`.
+    path_arc_centers
+        The center of the circle used in the Transform's radial path.
+
+        If this is set and path_func is not set, then a path_along_circles path will be generated
+        using the path_arc parameters and stored in path_func. If path_func is set, this and the
+        other path_arc fields are set as attributes, but a path_func is not generated.
+    replace_mobject_with_target_in_scene
+        Defines which object is replaced when the Transform is complete.
+
+        If set to True, the mobject will be removed from the scene and the target_mobject will
+        replace it. Otherwise, the target_mobject will be removed and the mobject will replace it.
+    """
+
     def __init__(
         self,
         mobject: Mobject | None,
@@ -61,14 +90,15 @@ def __init__(
         self.path_arc_centers: np.ndarray = path_arc_centers
         self.path_arc: float = path_arc
 
-        if self.path_arc_centers is not None:
+        if path_func is not None:
+            self.path_func: Callable = path_func
+        elif self.path_arc_centers is not None:
             self.path_func = path_along_circles(
                 path_arc,
                 self.path_arc_centers,
                 self.path_arc_axis,
             )
 
-        self.path_func: Callable | None = path_func
         self.replace_mobject_with_target_in_scene: bool = (
             replace_mobject_with_target_in_scene
         )
diff --git a/tests/test_graphical_units/control_data/transform/TransformWithConflictingPaths.npz b/tests/test_graphical_units/control_data/transform/TransformWithConflictingPaths.npz
new file mode 100644
index 0000000000..541684c6f9
Binary files /dev/null and b/tests/test_graphical_units/control_data/transform/TransformWithConflictingPaths.npz differ
diff --git a/tests/test_graphical_units/control_data/transform/TransformWithPathArcCenters.npz b/tests/test_graphical_units/control_data/transform/TransformWithPathArcCenters.npz
new file mode 100644
index 0000000000..3a0bc9a554
Binary files /dev/null and b/tests/test_graphical_units/control_data/transform/TransformWithPathArcCenters.npz differ
diff --git a/tests/test_graphical_units/control_data/transform/TransformWithPathFunc.npz b/tests/test_graphical_units/control_data/transform/TransformWithPathFunc.npz
new file mode 100644
index 0000000000..541684c6f9
Binary files /dev/null and b/tests/test_graphical_units/control_data/transform/TransformWithPathFunc.npz differ
diff --git a/tests/test_graphical_units/test_transform.py b/tests/test_graphical_units/test_transform.py
index aa13c3dcc9..aa5e740470 100644
--- a/tests/test_graphical_units/test_transform.py
+++ b/tests/test_graphical_units/test_transform.py
@@ -163,3 +163,39 @@ def test_ReplacementTransform(scene):
     v2 = Vector()
     v3 = Line()
     scene.play(ReplacementTransform(VGroup(v1, v2), v3))
+
+
+@frames_comparison(last_frame=False)
+def test_TransformWithPathFunc(scene):
+    dots_start = VGroup(*[Dot(LEFT, color=BLUE), Dot(3 * RIGHT, color=RED)])
+    dots_end = VGroup(*[Dot(LEFT + 2 * DOWN, color=BLUE), Dot(2 * UP, color=RED)])
+    scene.play(Transform(dots_start, dots_end, path_func=clockwise_path()))
+
+
+@frames_comparison(last_frame=False)
+def test_TransformWithPathArcCenters(scene):
+    dots_start = VGroup(*[Dot(LEFT, color=BLUE), Dot(3 * RIGHT, color=RED)])
+    dots_end = VGroup(*[Dot(LEFT + 2 * DOWN, color=BLUE), Dot(2 * UP, color=RED)])
+    scene.play(
+        Transform(
+            dots_start,
+            dots_end,
+            path_arc=2 * PI,
+            path_arc_centers=ORIGIN,
+        )
+    )
+
+
+@frames_comparison(last_frame=False)
+def test_TransformWithConflictingPaths(scene):
+    dots_start = VGroup(*[Dot(LEFT, color=BLUE), Dot(3 * RIGHT, color=RED)])
+    dots_end = VGroup(*[Dot(LEFT + 2 * DOWN, color=BLUE), Dot(2 * UP, color=RED)])
+    scene.play(
+        Transform(
+            dots_start,
+            dots_end,
+            path_func=clockwise_path(),
+            path_arc=2 * PI,
+            path_arc_centers=ORIGIN,
+        )
+    )


Code review comments:
diff_hunk: @@ -46,6 +46,35 @@
 
 
 class Transform(Animation):
+    """A Transform transforms a Mobject into a target Mobject.
+
+    Parameters
+    ----------
+    mobject
+        The Mobject to be transformed. The Mobject will be mutated into the target.
+    target_mobject
+        The Mobject that will be transformed into. Will become invisible and replaced by mobject.
+    path_func
+        A function defining the path that the mobject will take to target_mobject,
+        see `manim.utils.paths`.
+    path_arc
+        The arc (in radians) that the mobject will take to reach target if using a circular path
+        arc, see `path_arc_centers`.
+    path_arc_axis
+        The axis to rotate along if using a circular path arc, see `path_arc_centers`.
+    path_arc_centers
+        The center of the circle used in the Transform's radial path.
path: manim/animation/transform.py
text: @user1:
This parameter is still confusing to me; at the very least it is named terribly. Oh well.

```suggestion
        The center of the circular arcs along which the points of ``mobject`` are
        moved by the transformation.
```

diff_hunk: @@ -46,6 +46,35 @@
 
 
 class Transform(Animation):
+    """A Transform transforms a Mobject into a target Mobject.
+
+    Parameters
+    ----------
+    mobject
+        The Mobject to be transformed. The Mobject will be mutated into the target.
+    target_mobject
+        The Mobject that will be transformed into. Will become invisible and replaced by mobject.
path: manim/animation/transform.py
text: @user1:
The "Will become invisible"-part depends on the keyword argument passed to `replace_mobject_with_target_in_scene`; the current wording also suggests that it is visible at any point, which it is not.

```suggestion
        The target of the transformation.
```

diff_hunk: @@ -46,6 +46,35 @@
 
 
 class Transform(Animation):
+    """A Transform transforms a Mobject into a target Mobject.
+
+    Parameters
+    ----------
+    mobject
+        The Mobject to be transformed. The Mobject will be mutated into the target.
+    target_mobject
+        The Mobject that will be transformed into. Will become invisible and replaced by mobject.
+    path_func
+        A function defining the path that the mobject will take to target_mobject,
+        see `manim.utils.paths`.
+    path_arc
+        The arc (in radians) that the mobject will take to reach target if using a circular path
+        arc, see `path_arc_centers`.
+    path_arc_axis
+        The axis to rotate along if using a circular path arc, see `path_arc_centers`.
+    path_arc_centers
+        The center of the circle used in the Transform's radial path.
+
+        If this is set and path_func is not set, then a path_along_circles path will be generated
+        using the path_arc parameters and stored in path_func. If path_func is set, this and the
+        other path_arc fields are set as attributes, but a path_func is not generated.
+    replace_mobject_with_target_in_scene
+        Defines which object is replaced when the Transform is complete.
+
+        If set to True, the mobject will be removed from the scene and the target_mobject will
+        replace it. Otherwise, the target_mobject will be removed and the mobject will replace it.
path: manim/animation/transform.py
text: @user1:
Another minor correction regarding the actual behavior of the animation.

```suggestion
        Controls which mobject is replaced when the transformation is complete.

        If set to True, ``mobject`` will be removed from the scene and ``target_mobject`` will
        replace it. Otherwise, ``target_mobject`` is never added and ``mobject`` just takes its shape.
```