ManimCommunity/manim

Table.get_entries() should be split in two

Open

#2051 opened on Sep 16, 2021

View on GitHub
 (2 comments) (0 reactions) (0 assignees)Python (17,820 stars) (1,378 forks)batch import
enhancementgood first issue

Description

Currently there is this method:

    def get_entries(
        self, pos: Optional[Sequence[int]] = None
    ) -> Union[VMobject, VGroup]:
        """Return the individual entries of the table (including labels) or one specific entry
        if the parameter, ``pos``,  is set.

It would be much better if this were two separate functions:

    def get_entries(self) -> VGroup:
        ...

    def get_entry(self, pos:Sequence[int]) -> VMobject:
        ...

More obvious what is happening, simpler, you don't have the weirdness of get_entries returning a single entry, and it makes the return type explicit.

The code even makes it obvious that this is really two methods merged into one:

        if pos is not None:
            # One method here.
            if (
                self.row_labels is not None
                and self.col_labels is not None
                and self.top_left_entry is None
            ):
                index = len(self.mob_table[0]) * (pos[0] - 1) + pos[1] - 2
                return self.elements[index]
            else:
                index = len(self.mob_table[0]) * (pos[0] - 1) + pos[1] - 1
                return self.elements[index]
        else:
            # A completely different one here.
            return self.elements

Same for get_entries_without_labels.

I assume since you're still at version 0.10 breaking changes are allowed?

Contributor guide