Skip to content

Fix files with few MyPy typing errors #4263

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

Merged

Conversation

henrikmidtiby
Copy link
Contributor

Overview: What does this pull request change?

In this pull request I have addressed some of the files with only a few reported mypy errors.

Further Information and Comments

To identify files with few mypy errors, I cleared the mypy.ini file so no detected errors would be ignored and then used the follwing command

uv run pre-commit run mypy --all-files | grep "error" | sed 's/:.*//' | uniq -c | sort -n -r

I hope that this approach will make it easier to deal with the large files that needs a lot of type annotations added.

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

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 your contribution! I left some change requests:

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Dev Board Jun 22, 2025
henrikmidtiby and others added 10 commits June 22, 2025 22:24
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Cleaner way to indicate the float type

Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
@henrikmidtiby henrikmidtiby requested a review from chopan050 June 22, 2025 20:56
@henrikmidtiby
Copy link
Contributor Author

Thanks for your detailed comments. I have addressed all of the raised issues, in most cases by accepting your suggestions.

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!

I missed some last small things in mypy.ini. Could you please check them as well? After that, we should be ready to approve and merge this!

I also noticed that you modified a lot of files, but only some of them are indicated in mypy.ini for MyPy to stop ignoring their errors. Maybe you would like to add some more files in there? In particular, manim.animation.numbers, manim.scene.moving_camera_scene and manim.scene.section seem fine to me and MyPy didn't return any errors after adding them to mypy.ini, although manim.mobject.logo and the other animations you didn't add did throw some errors.

henrikmidtiby and others added 5 commits June 23, 2025 07:47
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
Co-authored-by: Francisco Manríquez Novoa <49853152+chopan050@users.noreply.github.com>
@chopan050 chopan050 enabled auto-merge (squash) June 23, 2025 11:58
@chopan050 chopan050 changed the title Fixing files with few typing (mypy) errors Fix files with few MyPy typing errors Jun 23, 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.

LGTM!

@chopan050 chopan050 merged commit 325fa08 into ManimCommunity:main Jun 23, 2025
21 of 22 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to 👍 To be merged in Dev Board Jun 23, 2025
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Jun 23, 2025
@henrikmidtiby henrikmidtiby deleted the FixingFilesWithFewTypingErrors branch June 24, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants