ManimCommunity/manim

Table.get_entries() should be split in two

Open

#2,051 opened on Sep 16, 2021

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

Repository metrics

Stars
 (17,820 stars)
PR merge metrics
 (Avg merge 90d 10h) (31 merged PRs in 30d)

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