Allow VGroup type subscripting#3606
Allow VGroup type subscripting#3606JasonGrace2282 wants to merge 19 commits intoManimCommunity:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
What are you trying to achieve with this feature? |
I agree that would be the best solution, but I wanted to avoid (if possible) having to manually make every class a
Oh I didn't know that, thanks for letting me know :) |
|
Having class VGroup(VMobject, Generic[T], metaclass=ConvertToOpenGL):
def __init__(self, *vmobjects: T, **kwargs):
super().__init__(**kwargs)
self.add(*vmobjects)But it would imply
Not sure I understood this part, could you please give an example? |
It is not in fact the case, but maybe something like
Sure, ideally we would only change what class VGroup(VMobject, Generic[T], metaclass=ConvertToOpenGL):
def __init__(self, *vmobjects: T, **kwargs):
super().__init__(**kwargs)
self.add(*vmobjects)
class Subclass1(VGroup):
pass
class Subclass2(VGroup[T]):
pass
Subclass1[VMobject] # error
Subclass2[VMobject] # no errorIt was just me being lazy and not wanting to have to create a |
|
I see. While this might look cumbersome to add the type variable to every subclass, this is required for type checkers to understand the generic class. Adding a bare In my opinion, the type variable should only be added to
You can do something like: VMobjectT = TypeVar("VMobjectT", bound=VMobject)
class VGroup(VMobject, Generic[VMobjectT], metaclass=ConvertToOpenGL):
def __init__(self, *vmobjects: VMobjectT, **kwargs): ...That way, users that use different vmobjects in a vgroup won't be impacted: g = VGroup(DashedVMobject(), DashedVMobject())
reveal_type(g) # VGroup[DashedVMobject()]
g = VGroup(DashedVMobject(), Triangle())
reveal_type(g) # "VGroup[DashedVMobject | Triangle]" for pyright | "VGroup[VMobject]" for mypyWe could also make use of PEP 696 in this case. It is still in draft but will most probably make it: from typing_extensions import TypeVar
VMobjectT = TypeVar("VMobjectT", bound=VMobject, default=VMobject)That way a plain As you can see, it is not perfect as mypy and pyright handle unions differently. But that could still work |
|
And I assume all subclasses would be subclassing class ManimBanner(VGroup[VMobject]):
... |
If the subclass behaves the same way |
.Mobject.__class_getitem__ to allow type subscripting|
Seeing as PEP 696 was accepted I've gone ahead and made the changes, let me know what you think! |
9caa588 to
99c3241
Compare
|
this week I'll have time to review your typing related PRs. |
d4a45d2 to
1d895b2
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| from manim.mobject.types.vectorized_mobject import VGroup | ||
| from manim.utils.color import BLACK, RED, YELLOW, ManimColor, ParsableManimColor | ||
|
|
||
| if TYPE_CHECKING: |
Check failure
Code scanning / CodeQL
Module-level cyclic import
| from manim.mobject.geometry.arc import Dot | ||
| from manim.mobject.svg.svg_mobject import SVGMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject, VMobjectT |
Check failure
Code scanning / CodeQL
Module-level cyclic import
| from manim.mobject.geometry.arc import Dot | ||
| from manim.mobject.svg.svg_mobject import SVGMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject, VMobjectT |
Check failure
Code scanning / CodeQL
Module-level cyclic import
| from manim.mobject.geometry.arc import Dot | ||
| from manim.mobject.svg.svg_mobject import SVGMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject | ||
| from manim.mobject.types.vectorized_mobject import VGroup, VMobject, VMobjectT |
Check failure
Code scanning / CodeQL
Module-level cyclic import
Allows things like
VGroup[Rectangle][v1]
Adds a
__class_getitem__method toMobjectThis was not useful, as discussed later
[v2]
Make
VGroupa genericGuideline
Since
VGroup's are invariant, typeVGroup's similar to lists. For example:Correct:
Incorrect:
Also renders correctly in the docs, ex LinearTransformationScene.get_ghost_vectors