Fixed issue when an animation is cached, manim can't merge the partial movie files. #1192

huguesdevimeux · 2021-03-29T15:19:03Z

Motivation

Fixed bug when a scene was already cached, manim would throw an error :

Overview / Explanation for Changes

It was just a piece of the logic of play that was broken.

For PRs introducing new features, please provide code snippets
using the newly introduced functionality and ideally even the
expected rendered output. -->

Testing Status

Tests pass.
Added a bunch of test to test the behavior of manim depending on what's cached.

Acknowledgements

I have read the Contributing Guidelines
I have chosen a descriptive PR title (see top of PR template for examples)

Reviewer Checklist

Newly added functions/classes are either private or have a docstring
Newly added functions/classes have tests added and (optional) examples in the docs
Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
The PR title is descriptive enough

… fixed-cached-probleem

TRoboto · 2021-03-29T19:43:20Z

LGTM!

manim/renderer/cairo_renderer.py

Co-authored-by: Darylgolden <[email protected]>

…im into fixed-cached-probleem

huguesdevimeux · 2021-03-30T12:00:58Z

Tests are misterioulsy failing on windows :/

huguesdevimeux · 2021-03-30T13:40:12Z

Flake tests are failing, I don't know why.

this PR is done from my point of view nevertheless, please review !

naveen521kk

sgtm

tests/assert_utils.py

manim/renderer/cairo_renderer.py

naveen521kk · 2021-03-30T14:40:54Z

Flake tests are failing, I don't know why.

That's because #1114 didn't have flake tests when merging. I think we should ask admins to edit branch settings that the branch needs to be updated before merging.

Co-authored-by: Naveen M K <[email protected]>

huguesdevimeux · 2021-03-30T20:57:29Z

Flake tests are failing, I don't know why.

That's because #1114 didn't have flake tests when merging. I think we should ask admins to edit branch settings that the branch needs to be updated before merging.

I see, thanks !

Co-authored-by: Naveen M K <[email protected]>

naveen521kk · 2021-03-31T14:53:34Z

Merging.

1) There's a new file for each option group 2) render is now a cloup.Command, not a Group Fixed issue when an animation is cached, manim can't merge the partial movie files. (ManimCommunity#1192) * fixed issue * fixed tests * Update manim/renderer/cairo_renderer.py Co-authored-by: Darylgolden <[email protected]> * added tests * imrpoved test * fixed logic * added new test * check if the file has been outputed * added test when caching is enabled * fixed tests on windows * black * Update manim/renderer/cairo_renderer.py Co-authored-by: Naveen M K <[email protected]> * Update tests/assert_utils.py Co-authored-by: Naveen M K <[email protected]> Co-authored-by: KingWampy <[email protected]> Co-authored-by: Darylgolden <[email protected]> Co-authored-by: Naveen M K <[email protected]> Added :ref_methods: to the manim directive (ManimCommunity#1209) * fix manim_directive for methods * added ref_methods to Angle example * black * added new ref_methods references * sort out ref_functions vs ref_methods in examples.rst Co-authored-by: Jason Villanueva <[email protected]> Co-authored-by: Benjamin Hackl <[email protected]> Fixed issue when an animation is cached, manim can't merge the partial movie files. (ManimCommunity#1192) * fixed issue * fixed tests * Update manim/renderer/cairo_renderer.py Co-authored-by: Darylgolden <[email protected]> * added tests * imrpoved test * fixed logic * added new test * check if the file has been outputed * added test when caching is enabled * fixed tests on windows * black * Update manim/renderer/cairo_renderer.py Co-authored-by: Naveen M K <[email protected]> * Update tests/assert_utils.py Co-authored-by: Naveen M K <[email protected]> Co-authored-by: KingWampy <[email protected]> Co-authored-by: Darylgolden <[email protected]> Co-authored-by: Naveen M K <[email protected]> Added :ref_methods: to the manim directive (ManimCommunity#1209) * fix manim_directive for methods * added ref_methods to Angle example * black * added new ref_methods references * sort out ref_functions vs ref_methods in examples.rst Co-authored-by: Jason Villanueva <[email protected]> Co-authored-by: Benjamin Hackl <[email protected]> Fixed unnecessary args dict

… Argparse (#1013) * Added click dependency and command structure * Refactored code for separation of concerns * Shortened plugins command to plugin, added render options * first draft for render -h * First successful render using click * Cleaned main * Moved flush_cache to option, ran black * Removed argparse logic, scattered print statements * corrected tests, all passing * merge upstream * fixed test with click's clirunner * Fixed doctest configuration.rst * Temporarily add in main_utils * Removed main_utils.parse_args, used ManimConfig.digest_args * fixed progress bar * Fix jupyter * black * Fixed incorrectly merged merge conflict * updated README command.png image * updated configuration.rst expected output * Fixed test_plugins and config_file expected type * Refixed the jupyter fix * Apply 3/5 suggestions Remove stray print Improve readability of test code Added module docs for the subcommands * Updated `main` to `manim` for tests * Forced `file` positional argument to be Path type * Fixed main -> manim * Added libpango to linux dependency * Updated poetry.lock * Changed configuration.rst test * Fixed test_a_flag test minor space issue added media_width to configuration.rst * Fixed fps flag in Cairo rendering * Fixed more outdated rst in sphinx docs Removed default for fps option, always overwrote quality Fixed doctest control_data * Fixed more incorrect rst orderings * Update tests/test_commands.py Co-authored-by: Naveen M K <[email protected]> * Added suggestions * Removed unused imports * Reverted entry point back to main * Update manim/_config/default.cfg Co-authored-by: Benjamin Hackl <[email protected]> * Adjusted ipython_magic's call to the entry_point * Converted frame_rate to int if integer * run black * Fixed doctest * Fixed issue with command name from CliRunner * Fixed multiple video windows opening from upstream merge * to black or not to black * Added deprecation warning to render subcommand * warning instead of warn * Applied Naveen's suggestions * Made `manim render` show the help page * Update manim/cli/render/commands.py Co-authored-by: Naveen M K <[email protected]> * Update manim/cli/cfg/commands.py Co-authored-by: Naveen M K <[email protected]> * Update manim/cli/cfg/commands.py Co-authored-by: Naveen M K <[email protected]> * Update manim/cli/plugins/commands.py Co-authored-by: Naveen M K <[email protected]> * Addressed some style changes * add back in write_to_movie temporarily for OpenGL support * Removed sound flag, deprecated use_opengl_renderer, added renderer option * revert webgl_renderer_path removal * Fixed cfg export Fixed readme usage of CLI * Flake8/black * Fixed bug in setting renderer choice * Removed log message due to default option Removed default option of background color Fixed write_to_movie flag default * Fix log_to_file tests * Make '-c' option for config_file, not background_color * print colored version always * Remove -v = --version shorthand, conflicts with verbosity * Use subprocess.run instead of Click's CliRunner for stdout * Refactor cli/render to use Cloup instead of click-option-group 1) There's a new file for each option group 2) render is now a cloup.Command, not a Group Fixed issue when an animation is cached, manim can't merge the partial movie files. (#1192) * fixed issue * fixed tests * Update manim/renderer/cairo_renderer.py Co-authored-by: Darylgolden <[email protected]> * added tests * imrpoved test * fixed logic * added new test * check if the file has been outputed * added test when caching is enabled * fixed tests on windows * black * Update manim/renderer/cairo_renderer.py Co-authored-by: Naveen M K <[email protected]> * Update tests/assert_utils.py Co-authored-by: Naveen M K <[email protected]> Co-authored-by: KingWampy <[email protected]> Co-authored-by: Darylgolden <[email protected]> Co-authored-by: Naveen M K <[email protected]> Added :ref_methods: to the manim directive (#1209) * fix manim_directive for methods * added ref_methods to Angle example * black * added new ref_methods references * sort out ref_functions vs ref_methods in examples.rst Co-authored-by: Jason Villanueva <[email protected]> Co-authored-by: Benjamin Hackl <[email protected]> Fixed issue when an animation is cached, manim can't merge the partial movie files. (#1192) * fixed issue * fixed tests * Update manim/renderer/cairo_renderer.py Co-authored-by: Darylgolden <[email protected]> * added tests * imrpoved test * fixed logic * added new test * check if the file has been outputed * added test when caching is enabled * fixed tests on windows * black * Update manim/renderer/cairo_renderer.py Co-authored-by: Naveen M K <[email protected]> * Update tests/assert_utils.py Co-authored-by: Naveen M K <[email protected]> Co-authored-by: KingWampy <[email protected]> Co-authored-by: Darylgolden <[email protected]> Co-authored-by: Naveen M K <[email protected]> Added :ref_methods: to the manim directive (#1209) * fix manim_directive for methods * added ref_methods to Angle example * black * added new ref_methods references * sort out ref_functions vs ref_methods in examples.rst Co-authored-by: Jason Villanueva <[email protected]> Co-authored-by: Benjamin Hackl <[email protected]> Fixed unnecessary args dict * Fixed bug that changed caching hashing result * Revert doctest logic for fps filename output Co-authored-by: Naveen M K <[email protected]> Co-authored-by: Benjamin Hackl <[email protected]> Co-authored-by: Gianluca Gippetto <[email protected]>

huguesdevimeux added 2 commits March 29, 2021 17:16

fixed issue

ccd6a3f

Merge branch 'master' of https://github.com/ManimCommunity/manim into…

d0c3aa9

… fixed-cached-probleem

huguesdevimeux added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Mar 29, 2021

huguesdevimeux changed the title ~~Fixed issue when an animation is cached~~ Fixed issue when an animation is cached, manim can't merge the partial movie files. Mar 29, 2021

fixed tests

52c1a4e

Darylgolden requested changes Mar 30, 2021

View reviewed changes

manim/renderer/cairo_renderer.py Outdated Show resolved Hide resolved

Update manim/renderer/cairo_renderer.py

94f8a0d

Co-authored-by: Darylgolden <[email protected]>

huguesdevimeux added this to the Release v0.5.0 milestone Mar 30, 2021

huguesdevimeux added 7 commits March 30, 2021 11:45

added tests

5ccf19b

Merge branch 'fixed-cached-probleem' of github.com:huguesdevimeux/man…

19208eb

…im into fixed-cached-probleem

imrpoved test

7131e91

fixed logic

e4049da

added new test

099a779

check if the file has been outputed

1b64d9d

added test when caching is enabled

bb167b8

huguesdevimeux added 2 commits March 30, 2021 14:37

fixed tests on windows

ad59d8f

black

5ca1738

huguesdevimeux marked this pull request as ready for review March 30, 2021 13:11

naveen521kk approved these changes Mar 30, 2021

View reviewed changes

tests/assert_utils.py Outdated Show resolved Hide resolved

naveen521kk reviewed Mar 30, 2021

View reviewed changes

manim/renderer/cairo_renderer.py Outdated Show resolved Hide resolved

Update manim/renderer/cairo_renderer.py

3d16b74

Co-authored-by: Naveen M K <[email protected]>

huguesdevimeux and others added 3 commits March 30, 2021 22:57

Merge branch 'master' into fixed-cached-probleem

2c07bb6

Update tests/assert_utils.py

ed293ac

Co-authored-by: Naveen M K <[email protected]>

Merge branch 'master' into fixed-cached-probleem

3ab89e1

naveen521kk merged commit 21ad14e into ManimCommunity:master Mar 31, 2021

huguesdevimeux deleted the fixed-cached-probleem branch March 31, 2021 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed issue when an animation is cached, manim can't merge the partial movie files. #1192

Fixed issue when an animation is cached, manim can't merge the partial movie files. #1192

Uh oh!

huguesdevimeux commented Mar 29, 2021 •

edited

Loading

Uh oh!

TRoboto commented Mar 29, 2021

Uh oh!

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

naveen521kk left a comment

Uh oh!

Uh oh!

Uh oh!

naveen521kk commented Mar 30, 2021

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

naveen521kk commented Mar 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fixed issue when an animation is cached, manim can't merge the partial movie files. #1192

Fixed issue when an animation is cached, manim can't merge the partial movie files. #1192

Uh oh!

Conversation

huguesdevimeux commented Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Overview / Explanation for Changes

Testing Status

Acknowledgements

Reviewer Checklist

Uh oh!

TRoboto commented Mar 29, 2021

Uh oh!

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

naveen521kk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

naveen521kk commented Mar 30, 2021

Uh oh!

huguesdevimeux commented Mar 30, 2021

Uh oh!

naveen521kk commented Mar 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

huguesdevimeux commented Mar 29, 2021 •

edited

Loading