Skip to content
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

Fix wrong value of MObject width/height with empty VGroup #4088

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

gvauvert
Copy link

@gvauvert gvauvert commented Jan 2, 2025

Overview: What does this pull request change?

Fix the issue 4087.

When an MObject contains an empty VGroup, its width or height can be wrong

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

Bug fix + tests

Links to added or changed documentation pages

No documentation page, bug fix only.

Further Information and Comments

  • add test
  • fix bug
  • fix another test which result depends on the bug

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

@@ -136,21 +136,19 @@ def test_mobject_dimensions_nested_mobjects():


def test_mobject_dimensions_mobjects_with_no_points_are_at_origin():
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the previous test was testing an unexpected behavior, it should have been linked to an issue.

@@ -2149,10 +2153,11 @@ def get_nadir(self) -> Point3D:

def length_over_dim(self, dim: int) -> float:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this function is meant to be used only internally, it should be private

Suggested change
def length_over_dim(self, dim: int) -> float:
def __length_over_dim(self, dim: int) -> float:

or at least protected

Suggested change
def length_over_dim(self, dim: int) -> float:
def _length_over_dim(self, dim: int) -> float:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant