Enhance throw_error_if_no_points error message - #4927
Open
smmariquit wants to merge 2 commits into
Open
Conversation
behackl
requested changes
Aug 12, 2026
behackl
left a comment
Member
There was a problem hiding this comment.
Thanks for your contribution! There is a few small things I'd like to discuss before we get this merged.
- In the case where there is only one child the error reads "..., but 1 of its 1 submobjects do", which reads a bit weird. I'd be in favour of special-casing the 1-child situation.
- Checking direct children also means that nested mobjects like
VGroup(VGroup(Dot()))would not get a more helpful message. The question (that I don't really have a great answer to, would like to hear your opinion) is whether restricting the check to direct children is fine, or whether iterating over the entire family of a mobject would make more sense? - Specifying the raised exception is a good idea too, I'm not sure about
ValueErrorthough. Isn't this more of aRuntimeErrorsituation? - Tests please! I think there is a test for the old version of this exception already, this should be extended a bit to reflect the more detailed exception text.
Thanks again, let me know what you think!
Contributor
|
My $0.02:
|
Contributor
|
To challenge my own previous comment: Perhaps the "did you mean to pass..." message actually makes perfect sense if the method is explicitly called on a group-like mobject ( It is very uncommon for people to manually add points to group mobjects, so if such a mobject is passed to a point-requiring method, it's a good indication that something unintended is going on. This doesn't catch message = f"Cannot call {cls}.{caller_name} because {self} has no points."
if (
isinstance(self, self.get_group_class())
and (drawn := [m for m in self if m.has_points()])
):
message += (
f" It has {len(drawn)} submobjects with points, "
"did you mean to pass one of them?"
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #4921. Replaces the generic
Exceptionwith aValueErrorand explicitly names the class and points out if submobjects contain points. This is particularly helpful when aVGroupis accidentally passed, as the new error message clearly identifies it as a container with no points of its own.