Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Fixed a bug in regards to empty inputs in AddTextLetterByLetter class. #3404

Merged
merged 17 commits into from
Oct 26, 2023
Merged
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b9d2377
Misc: Just a class to test out some functions
Immanuel-Alvaro-Bhirawa Oct 10, 2023
73b6d07
Merge branch 'main' of https://github.com/Immanuel-Alvaro-Bhirawa/manim
Immanuel-Alvaro-Bhirawa Oct 10, 2023
cdd4903
Fix: Fixed a bug in AddTextLetterByLetter class
Immanuel-Alvaro-Bhirawa Oct 14, 2023
b89ffcc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 14, 2023
37698f0
Merge branch 'main' into main
Immanuel-Alvaro-Bhirawa Oct 14, 2023
e3f95db
Fix: Adjusted changes according to Ben's comments
Immanuel-Alvaro-Bhirawa Oct 18, 2023
db1fc4f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2023
d3f0a10
Fix: Removed imports
Immanuel-Alvaro-Bhirawa Oct 18, 2023
d1cc590
Merge branch 'main' of https://github.com/Immanuel-Alvaro-Bhirawa/manim
Immanuel-Alvaro-Bhirawa Oct 18, 2023
2b8bf5b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2023
42b2cb8
Merge branch 'main' into main
Immanuel-Alvaro-Bhirawa Oct 25, 2023
1159f65
Feat: Adjusted changes to AddTextLetterByLetter
Immanuel-Alvaro-Bhirawa Oct 25, 2023
72256fd
Merge branch 'main' of https://github.com/Immanuel-Alvaro-Bhirawa/manim
Immanuel-Alvaro-Bhirawa Oct 25, 2023
0c7ee00
Merge branch 'main' into main
behackl Oct 25, 2023
9d69ce2
Merge branch 'main' of https://github.com/Immanuel-Alvaro-Bhirawa/manim
Immanuel-Alvaro-Bhirawa Oct 25, 2023
0b442a2
Feat: Added test_creation
Immanuel-Alvaro-Bhirawa Oct 25, 2023
b638927
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions manim/animation/creation.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,15 +563,22 @@ def __init__(
**kwargs,
) -> None:
self.time_per_char = time_per_char

# Check for empty text using family_members_with_points()
if not text.family_members_with_points():
raise ValueError(
"The input text is empty i.e., input has no characters to read thus cannot render a video file."
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'd prefer a shorter error message (e.g., f"The text mobject {text} does not seem to contain any characters.") and remove the comment about the video file (which is a consequence the user does not strictly need to know about).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay got it Ben!

)

if run_time is None:
# minimum time per character is 1/frame_rate, otherwise
# the animation does not finish.
Immanuel-Alvaro-Bhirawa marked this conversation as resolved.
Show resolved Hide resolved
run_time = np.max((1 / config.frame_rate, self.time_per_char)) * len(text)

Copy link
Member

Choose a reason for hiding this comment

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

Any reason for adding this newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason at all!
I wasn't paying attention again. Completely My bad.

super().__init__(
text,
suspend_mobject_updating=suspend_mobject_updating,
int_func=int_func,
rate_func=rate_func,
time_per_char=time_per_char,
Copy link
Member

Choose a reason for hiding this comment

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

The parent class, ShowIncreasingSubsets does not seem to have a time_per_char keyword argument -- this should not be propagated upwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood got it!

run_time=run_time,
reverse_rate_function=reverse_rate_function,
introducer=introducer,
Expand Down
Loading