Skip to content

Conversation

@JasonGrace2282
Copy link
Member

@JasonGrace2282 JasonGrace2282 commented Aug 4, 2023

Overview: What does this pull request change?

  • Added an example to always_redraw
  • Changed lambda m: mob.become(func()) to lambda _: mob.become(func()) for clarity

Motivation and Explanation: Why and how do your changes improve the library?

There wasn't much of an example for always_redraw, which might've helped with learning how to use updators and such.

Links to added or changed documentation pages

https://manimce--3312.org.readthedocs.build/en/3312/reference/manim.animation.updaters.mobject_update_utils.html

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@JasonGrace2282 JasonGrace2282 changed the title Added Docstring & Example for always_redraw Added Docstring/Example for always_redraw Aug 4, 2023
@jsonvillanueva jsonvillanueva changed the title Added Docstring/Example for always_redraw Added Docstring/Example for :meth:always_redraw Aug 4, 2023
Copy link
Member

@jsonvillanueva jsonvillanueva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the doc snippet! The TangentAnimation gives two good examples of always_redraw. I would only shorten the run_time to make the animation render quicker during our Read the Docs (RTD) builds and also make the video size a bit smaller (most docs animations are short for this reason).

@jsonvillanueva jsonvillanueva added the documentation Improvements or additions to documentation label Aug 4, 2023
Co-authored-by: Jason Villanueva <[email protected]>
@JasonGrace2282
Copy link
Member Author

@jsonvillanueva I took your suggestion. Any other commends or todo's?

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thank you for your efforts! I've left some more comments, please have a look.

return mobject


def always_redraw(func):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we are at it, we could probably also add a type hint for func, it should be something like Callable[[], Mobject] (with Callable from collections.abc) I guess.

@JasonGrace2282
Copy link
Member Author

JasonGrace2282 commented Aug 5, 2023

@behackl I updated my PR to reflect your suggestions.

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it seems I failed to comment one final suggestion that I had that added an explicit parameter description -- and while fixing that, I also saw that the return type of the function was missing.

After this, I'm happy and ready to merge though; thank you!

@JasonGrace2282
Copy link
Member Author

@behackl Implemented your suggestions!

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again!

@behackl behackl merged commit 59ff327 into ManimCommunity:main Aug 6, 2023
@behackl behackl added this to the v0.18.0 milestone Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants