Axes coords_to_point should accept sequences

[k4pran]

Description of bug / unexpected behavior

The Axes class method coords_to_point states that it accepts a sequence when it doesn't, it only accepts a variable number of floats. This meants that you cannot do things like:

axes.coords_to_point(RIGHT)

and instead need to do unpack the sequence:

axes.coords_to_point(*RIGHT)

Expected behavior

I think it would be more consistent for it to accept sequences as this is more compatible with the rest of manim, and there is only ever going to be a max of 3 input values.

How to reproduce the issue

Code for reproducing the problem

class Example(Scene):

    def construct(self):
        axes = Axes(axis_config={'include_numbers': True})
        f_vec = Vector(axes.coords_to_point(*RIGHT))
        f_vec.set_stroke(width=2.0)
        self.add(axes, f_vec)

Additional media files

Images/GIFs

Logs

Terminal output

PASTE HERE OR PROVIDE LINK TO https://pastebin.com/ OR SIMILAR

System specifications

System Details

• OS (with version, e.g Windows 10 v2004 or macOS 10.15 (Catalina)):
• RAM:
• Python version (python/py/python3 --version):
• Installed modules (provide output from pip list):

PASTE HERE

LaTeX details

• LaTeX distribution (e.g. TeX Live 2020):
• Installed LaTeX packages:

FFMPEG

Output of ffmpeg -version:

PASTE HERE

Additional comments

[hydrobeam]

I agree. It's pretty annoying that c2p functions differently from other ways that points are defined. Unfortunately, this will be a breaking change... but it's better to get it done now than later.

Speaking of points, maybe having a check that there is only an x/y point defined would be helpful to accompany this change. Currently, passing in a point such as [x,y] errors out.

[k4pran]

I agree. It's pretty annoying that c2p functions differently from other ways that points are defined. Unfortunately, this will be a breaking change... but it's better to get it done now than later.

Speaking of points, maybe having a check that there is only an x/y point defined would be helpful to accompany this change. Currently, passing in a point such as [x,y] errors out.

It is a bit of a hack but you could avoid breaking compatibility by changing the signature to:

def coords_to_point(self, *coords: Union[int, float, Sequence[float]]) -> np.ndarray

Then checking if the first element is an int, float or sequence and if it is a sequence get the sequence:

coords = coords[0]

Would be better not to do that I think though.

"Speaking of points, maybe having a check that there is only an x/y point defined would be helpful to accompany this change. Currently, passing in a point such as [x,y] errors out"

Do you mean for this particular method it errors out? Seems ok when I do:

axes.coords_to_point(1, 1)

[hydrobeam]

What I meant was for points in general. It's a little annoying to have to specify a z-coordinate when you aren't operating in that axis. Here's an example:

class Example(Scene):
    def construct(self):
        a = Dot([1,2])
        self.add(a)

ValueError: operands could not be broadcast together with shapes (32,3) (2,) (32,3)

What I would expect is that it would default the z-value to 0. So [1,2] would automatically become [1, 2, 0].

Then checking if the first element is an int, float or sequence and if it is a sequence get the sequence:

I think this is a decent idea. We can add output a warning that they're using an outdated way of using c2p and mention that it'll be fully removed in one/two releases.

[k4pran]

What I would expect is that it would default the z-value to 0. So [1,2] would automatically become [1, 2, 0].

Ah yes I see, it would be an easy fix to just make sure that the list is a specific size, but maybe it would be better to have an Point class with x, y, z properties so that it is easier to enforce something like this across manim?

[MrDiver]

PR #2739