Skip to content

Conversation

fmuenkel
Copy link
Contributor

@fmuenkel fmuenkel commented Aug 5, 2025

  • Rename SceneFileWriter.save_final_image() to save_image() and fix docstring

  • fix two type annotations in scene_file_writer.py

  • make docstrings consistent

Overview: What does this pull request change?

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

save_final_image() was incorrectly named. It is simply saving an image at any point during the scene. It is used in cairo_renderer.py and opengl_renderer.py to save the last frame if save_last_frame is set in the configuration.

Links to added or changed documentation pages

Further Information and Comments

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

* Rename SceneFileWriter.save_final_image() to save_image() and fix
  docstring

* fix some type annotations

* make docstrings consistent
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.

Fine with me, thanks! Will leave open for a bit before merging in case someone else thinks that a deprecation period is necessary.

@github-project-automation github-project-automation bot moved this from 🆕 New to 👍 To be merged in Dev Board Aug 7, 2025
@behackl behackl added the refactor Refactor or redesign of existing code label Aug 7, 2025
Copy link
Contributor

@chopan050 chopan050 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 change! I agree with it.

There's one detail I want to talk about:

@github-project-automation github-project-automation bot moved this from 👍 To be merged to 👀 In review in Dev Board Aug 11, 2025
Co-authored-by: Francisco Manríquez Novoa <[email protected]>
@fmuenkel fmuenkel requested a review from chopan050 August 11, 2025 11:47
Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-project-automation github-project-automation bot moved this from 👀 In review to 👍 To be merged in Dev Board Aug 11, 2025
@behackl behackl merged commit f69f254 into ManimCommunity:main Aug 11, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Aug 11, 2025
@fmuenkel fmuenkel deleted the fix-scene-file-writer-typing branch August 11, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor or redesign of existing code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants