Mark some methods as internal by JasonGrace2282 · Pull Request #3793 · ManimCommunity/manim

JasonGrace2282 · 2024-05-30T17:29:40Z

This is an idea of an implementation for #3792

It defines a decorator that modifies the function's docstring to warn end-users that it is marked as an implementation detail.

Example

TODO

Figure out why sometimes the decorator does not produce the warning directive

manim/utils/decorators.py

behackl

This looks fine! Left one suggestion and a bunch of comments where I thought it is somewhat debatable that a method would be marked as internal.

I like the mechanism though, am happy to merge this in principle. Thanks for your work!

behackl · 2024-07-13T17:27:03Z

manim/mobject/mobject.py


        return self.rescale_to_fit(depth, 2, stretch=True, **kwargs)

+    @internal


To be honest, I don't remember. However, I think that it's more common to use move_to or set_(x|y|z) as opposed to this method.
This PR was mostly an introduction of concept, so I'm happy to remove it if you disagree.

behackl · 2024-07-13T17:27:12Z

manim/mobject/mobject.py

        )
        return self

+    @internal


There was already a set_colors_by_gradient that used this internally and returning Self, so I figured that should be the public one.

behackl · 2024-07-13T17:27:18Z

manim/mobject/mobject.py

            mob.set_color(color, family=False)
        return self

+    @internal


Here, there was already a set_colors_by_radial_gradient that used this internally and returning Self, so I figured that should be the public one.

behackl · 2024-07-13T17:35:24Z

manim/mobject/mobject.py

    def get_num_points(self) -> int:
        return len(self.points)

+    @internal


maybe also borderline

manim/mobject/mobject.py

behackl · 2024-07-13T17:36:39Z

manim/mobject/mobject.py

    def repeat_submobject(self, submob: Mobject) -> Self:
        return submob.copy()

+    @internal


borderline?

behackl · 2024-07-13T17:36:53Z

manim/mobject/types/vectorized_mobject.py

                submob.z_index_group = self
        return self

+    @internal


I mostly added this because of discussions on whether to make points a property (to add some memoization stuff).

behackl · 2024-07-13T17:39:40Z

manim/mobject/types/vectorized_mobject.py

        point.match_style(self)
        return point

+    @internal


borderline?

behackl · 2024-07-13T17:39:47Z

manim/mobject/types/vectorized_mobject.py

                    val = val.copy()
                setattr(self, attr, val)

+    @internal


borderline?

I find it hard to think of a case where a user would need this, and (I can't remember) but there's a chance this changes in experimental.

manim/utils/decorators.py

Co-authored-by: Benjamin Hackl <devel@benjamin-hackl.at>

JasonGrace2282 · 2024-07-14T19:49:19Z

Before this PR is merged, it might also be worth checking if there is a performance penalty.
Currently, this implementation does not take docstrings from parent classes (which is what inspect.getdoc does), as doing so is obviously slower - maybe there's a way we can only modify the docstrings while sphinx is building the docs?
EDIT: Environment variables might be the best way to go here - according to the os docs environment variables are stored in memory so it should just be a lookup away?

EDIT 2: Just did some benchmarking, the performance penalty from inspect.cleandoc is very low. Considering that it is only applied once at import time, it should be negligible.

JasonGrace2282 · 2024-07-14T19:57:21Z

it is somewhat debatable that a method would be marked as internal.

My view is always that when in doubt, mark it as internal. This gives us the most flexibility for the future (even though we're not at 1.0.0 yet!), and changing something from internal → public is not a major semvar change.

JasonGrace2282 added 2 commits May 30, 2024 13:30

Add decorator

8d7bb13

Mark some methods as internal on Mobject/VMobject

bfea7da

JasonGrace2282 force-pushed the internal branch from 09ee2df to bfea7da Compare May 30, 2024 17:30

JasonGrace2282 marked this pull request as draft May 30, 2024 17:48

Fix docstrings being weird and causing build warnings

bd7eae7

JasonGrace2282 marked this pull request as ready for review May 31, 2024 01:57

Viicos reviewed May 31, 2024

View reviewed changes

manim/utils/decorators.py Outdated Show resolved Hide resolved

JasonGrace2282 and others added 4 commits May 31, 2024 08:00

don't use ParamSpec

d5377cb

Merge branch 'main' into internal

b0df579

Change spacing

97b7a57

Merge branch 'main' into internal

0eddd98

JasonGrace2282 added this to the v0.19.0 milestone Jun 10, 2024

behackl reviewed Jul 13, 2024

View reviewed changes

JasonGrace2282 and others added 2 commits July 14, 2024 09:57

Improve decorator message

10fa509

Co-authored-by: Benjamin Hackl <devel@benjamin-hackl.at>

Some feedback

c6c6533

Merge branch 'main' into internal

82d39f2

JasonGrace2282 added enhancement Additions and improvements in general needs discussion Things which needs to be discussed before implemented. labels Jul 25, 2024

JasonGrace2282 modified the milestones: v0.19.0, v0.20.0 Jul 25, 2024

JasonGrace2282 removed the needs discussion Things which needs to be discussed before implemented. label Aug 30, 2024

chopan050 mentioned this pull request Jul 25, 2025

Fix wrong value of MObject width/height with empty VGroup #4088

Open

3 tasks


		return self.rescale_to_fit(depth, 2, stretch=True, **kwargs)

		@internal

Conversation

JasonGrace2282 commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

Uh oh!

behackl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JasonGrace2282 commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JasonGrace2282 commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JasonGrace2282 commented May 30, 2024 •

edited

Loading

JasonGrace2282 commented Jul 14, 2024 •

edited

Loading

JasonGrace2282 commented Jul 14, 2024 •

edited

Loading