Skip to content

Conversation

fmuenkel
Copy link
Contributor

Overview: What does this pull request change?

  • Add type hints to valuetracker.py
  • Add error handling for __iadd__() and __isub__()
  • Add __sub__() to solve mypy error for __isub__()
  • Small improvements to docstrings

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

Related to #3375

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

@fmuenkel fmuenkel force-pushed the typing-value-tracker branch from 1ab6b56 to af88adf Compare February 9, 2025 15:22
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! Thanks for these changes!

@chopan050 chopan050 changed the title Add type hints to mobject/valuetracker.py Add type hints to mobject/valuetracker.py Jul 25, 2025
@github-project-automation github-project-automation bot moved this to 🆕 New in Dev Board Jul 25, 2025
@chopan050 chopan050 enabled auto-merge (squash) July 25, 2025 16:55
@chopan050 chopan050 disabled auto-merge July 25, 2025 17:00
@chopan050 chopan050 changed the title Add type hints to mobject/valuetracker.py Add type hints to mobject/value_tracker.py Jul 25, 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.

Sorry - I did not notice the __sub__ method when I approved the PR earlier. I understand that MyPy requires it. However, there is something I have to say about the implementation:

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Dev Board Jul 25, 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! Thanks for the changes!

Now that __sub__ and __add__ is implemented, I noticed that we should do the same for __mul__, __truediv__, __floordiv__, __pow__ and __mod__, since their inplace counterparts are implemented. Now, since this is mainly a PR for adding type hints, this should be done in a subsequent PR. Would you like to do that PR as well?

Meanwhile, I'll just approve and merge this PR.

@github-project-automation github-project-automation bot moved this from 👀 In review to 👍 To be merged in Dev Board Jul 25, 2025
@chopan050 chopan050 enabled auto-merge (squash) July 25, 2025 21:20
@chopan050 chopan050 merged commit a8c16fb into ManimCommunity:main Jul 25, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Jul 25, 2025
@fmuenkel
Copy link
Contributor Author

Sure, I will have a look at it..

@fmuenkel fmuenkel deleted the typing-value-tracker branch July 25, 2025 22:44
@chopan050 chopan050 changed the title Add type hints to mobject/value_tracker.py Add type hints and support for arithmetic operators + and - on ValueTracker Jul 26, 2025
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