Add typings to tex_mobject.py and numbers.py
#4015
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| sub_families = [x.get_family() for x in self.submobjects] | ||
| all_mobjects = [self] + list(it.chain(*sub_families)) | ||
| return remove_list_redundancies(all_mobjects) | ||
|
|
||
| def family_members_with_points(self) -> list[Self]: | ||
| def family_members_with_points(self) -> Sequence[Self]: | ||
| return [m for m in self.get_family() if m.get_num_points() > 0] |
There was a problem hiding this comment.
Isn't the usual recommendation to have type annotations for function inputs be as general as possible and have type annotations for function outputs be as specific as possible? It's similar to Postel's law: "be conservative in what you send, be liberal in what you accept". So, I think the return types here should be list[...].
There was a problem hiding this comment.
That's generally very true, but there's an issue with returning, say, a list[Mobject]: subclasses like VMobject cannot be typehinted to return a list[VMobject], because both lists are incompatible, even though VMobject is a subclass of Mobject. This does not happen when we use Sequence, which makes it the most specific type without that issue.
Therefore, the rule we apply here is:
- If it's a method of a class meant to be subclassed, use
Sequence. - Otherwise, use
list. There are some plain functions, not methods, which are typehinted to return alist.
There was a problem hiding this comment.
I'm not sure that this logic applies here, because of the usage of Self. From a quick test it seems that we should be able to do it with list.
There was a problem hiding this comment.
I had to use list because of an error MyPy threw. Somewhere, I had a list of SingleStringMathTex and MyPy couldn't let me append a MathTex. I'll take a look again to see if there's another way of solving this, though.
| self.name: str = self.__class__.__name__ if name is None else name | ||
| self.dim: int = dim | ||
| self.target: Mobject | None = target | ||
| self.z_index: float = z_index |
There was a problem hiding this comment.
It's kind of weird that mypy doesn't infer these? It should be able to 🤔
There was a problem hiding this comment.
It should be able to infer them. I've just seen a lot of attribute typehints around the source code, so I thought I should do the same here.
| sub_families = [x.get_family() for x in self.submobjects] | ||
| all_mobjects = [self] + list(it.chain(*sub_families)) | ||
| return remove_list_redundancies(all_mobjects) | ||
|
|
||
| def family_members_with_points(self) -> list[Self]: | ||
| def family_members_with_points(self) -> Sequence[Self]: | ||
| return [m for m in self.get_family() if m.get_num_points() > 0] |
There was a problem hiding this comment.
I'm not sure that this logic applies here, because of the usage of Self. From a quick test it seems that we should be able to do it with list.
| self.number: float | complex = number | ||
| self.num_decimal_places: int = num_decimal_places | ||
| self.include_sign: bool = include_sign | ||
| self.mob_class: type[TextLike] = mob_class | ||
| self.group_with_commas: bool = group_with_commas | ||
| self.digit_buff_per_font_unit: float = digit_buff_per_font_unit | ||
| self.show_ellipsis: bool = show_ellipsis | ||
| self.unit: str | None = unit | ||
| self.unit_buff_per_font_unit: float = unit_buff_per_font_unit | ||
| self.include_background_rectangle: bool = include_background_rectangle | ||
| self.edge_to_fix: Vector3D = edge_to_fix | ||
| self._font_size: float = font_size | ||
| self.fill_opacity: float = fill_opacity |
There was a problem hiding this comment.
Again, this feels a little bit suspicious - mypy should be able to figure out types from assignment?
Important remarks:
VMobject.color, even when I add typings toMobject.colorandOpenGLMobject.color. For this reason, I added# type: ignore [has-type]comments every time the.colorattribute was used.TextLiketype alias. I didn't know how to make it aTypeVarfor a more proper usage inDecimalNumber._string_to_mob(), because MyPy always reported aType variable TextLike is unbounderror, and I couldn't cover the different possible cases. Any suggestions are welcome.Reviewer Checklist