manim icon indicating copy to clipboard operation
manim copied to clipboard

Add type annotations to `manim/_config/utils.py`

Open henrikmidtiby opened this issue 9 months ago • 5 comments

Overview: What does this pull request change?

More work on https://github.com/ManimCommunity/manim/issues/3375

Continuation of PR #4134

The module manim/_config/utils.py is now going through mypy checks without raising any issues.

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

henrikmidtiby avatar Apr 27 '25 19:04 henrikmidtiby

@JasonGrace2282 Would you take a look at this PR?

henrikmidtiby avatar Apr 27 '25 21:04 henrikmidtiby

Since we merged PR #4363 which typed opengl_renderer_window.py and stopped ignoring its errors, there was a new error raising in there because of the type of the window_size parameter of Window. I made a commit which fixes that error by allowing window_size to be a tuple of ints and further checking its values.

chopan050 avatar Aug 11 '25 18:08 chopan050

I just saw @behackl comment. Before reading it, I already committed a fix 😅

Suggestion (up for discussion of course): we could rewrite the setter of config.window_size to be tuple[int, int] | None, with None taking over the current role of 'default', and then simplify the logic in opengl_renderer_window.py. Thoughts?

Would that also make the getter of config.window_size return tuple[int, int] | None? If so, that would be a nice change. I like the idea.

Actually, instead of having too much logic for validating and parsing window_size in Window, there should probably also be a parsing logic inside config.window_size's setter.

EDIT: this should all probably be done in a follow-up PR

chopan050 avatar Aug 11 '25 19:08 chopan050

I would suggest to move all the interpretation of the windows_size value into the config file and then skip parsing these values in the constructor for the Window class in the opengl_renderer_window.py file. I can try to do that in a separate PR.

henrikmidtiby avatar Aug 16 '25 15:08 henrikmidtiby