Skip to content

Cleanup TipableVMobject: avoid mutable default and fix assign_tip_attr typo#4503

Merged
behackl merged 4 commits intoManimCommunity:mainfrom
josiest:tipable-object-dependency-cleanup
Feb 22, 2026
Merged

Cleanup TipableVMobject: avoid mutable default and fix assign_tip_attr typo#4503
behackl merged 4 commits intoManimCommunity:mainfrom
josiest:tipable-object-dependency-cleanup

Conversation

@josiest
Copy link
Contributor

@josiest josiest commented Dec 5, 2025

Overview: What does this pull request change?

  • tip_style parameter in TipableVMobject constructor now defaults to None instead of a mutable dict
  • Fixed a typo in TipableVMobject.assign_tip_attr method name
  • Added some TODOs to outline my plan to fix some issues with how TipableVMobject adds new tips

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

Just a few touch ups I thought I'd make a pull request for:

  1. The TipableVMobject constructor's tip_style parameter had a mutable default argument (see here for why this is likely not what the original author of this constructor intended)
  • Not sure if this causes any bugs rn, but it could very well in the future. Better to nip in the bud and change it since I caught it
  • I just implemented the standard fix: pass None as default argument instead, and default the attribute to an empty dict if given None
  1. TipableVMobject.asign_tip_attr has a typo in the method name. Seems to only be used in one place so it's an easy fix
  2. After working on a custom tip shape, I ran into a few unexpected behaviors. I've posted an in-depth analysis in this thread.
  • While the TipableVMobject class isn't broken, these issues cause hiccups and unexpected behavior when developing new tip shapes.
  • I've outlined in some TODO comments how I believe these issues can be fixed with minimal changes to existing code. I fully intend to fix these issues myself, but I thought it might be useful to merge into upstream to share my progress in case life gets in the way.

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

- fixed mutable default argument in Tipable VMobject constructor
- fixed typo in method name assign_tip_attr
- added two todo's which outline unexpected behavior in tip placement and how to solve
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.

Hi, thanks for your contribution! Two things:

  • the renamed method for fixing the typo, there is another one in opengl_geometry.py -- please update it there as well,
  • and instead of adding TODOs in comments to the code base, please open a corresponding issue instead!

Other than that, this seems good to go from my side. Thanks again!

@behackl
Copy link
Member

behackl commented Feb 22, 2026

I picked this up and addressed the outstanding review points:

This branch now contains the changes.

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.

Merging this once tests pass.

@behackl behackl added the enhancement Additions and improvements in general label Feb 22, 2026
@behackl behackl changed the title Small touch ups before fixing bugs with tip rendering Cleanup TipableVMobject: avoid mutable default and fix assign_tip_attr typo Feb 22, 2026
@behackl behackl changed the title Cleanup TipableVMobject: avoid mutable default and fix assign_tip_attr typo Cleanup TipableVMobject: avoid mutable default and fix assign_tip_attr typo Feb 22, 2026
@behackl behackl merged commit a6af7f3 into ManimCommunity:main Feb 22, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Additions and improvements in general

Projects

Status: 🆕 New

Development

Successfully merging this pull request may close these issues.

2 participants