Add type annotations to manim/_config/utils.py

[henrikmidtiby]

Overview: What does this pull request change?

More work on #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]

@JasonGrace2282 Would you take a look at this PR?

[behackl]

Seems fine to me, thank you very much!

[behackl]

Upon closer inspection (and actually due to our pre-commit mypy check): there now is an issue with the config.window_size key typing that revealed a bug:

The OpenGLWindow does expect config.window_size to be a string of shape 'width,height', or 'default'. If one sets config.window_size = (900, 600) or something like that (which is actually quite reasonable to assume, that an integer-tuple is a suitable choice), then the window crashes because it tries to call .split on the config value.

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?

[chopan050]

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]

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

[henrikmidtiby]

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.

stale

[chopan050]

I noticed that, in the last commit, the utils MyPy errors were ignored again. Was that intended? After stopping ignoring them on my machine, a lot of errors appeared. Almost all of them are "Incompatible return value type" because they depend on self._d, a dictionary whose keys are typed as Any | None.

[behackl]

So, whats the latest status here? As per https://github.com/henrikmidtiby/manim/blob/9157452bd7120a0cbd6403cf78115249f92953e0/mypy.ini#L55-L56 the mypy errors are currently ignored in this file. I'm happy to merge this cleanup as-is if messing with the config dict is too tiring. (I'm heavily contemplating staring with pydantic-fying the config rather sooner than later, so not sure how much effort we should still put into the current state of affairs.)

[henrikmidtiby]

I would suggest to merge this issue now and then deal with the configuration changes in PR #4399

[chopan050]

I would suggest to merge this issue now and then deal with the configuration changes in PR #4399

Sure!