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

Added missing default_numbers_to_display() in NumberLine Class #4028

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zoya5636
Copy link

Overview: What does this pull request change?

I added a default_numbers_to_display function in the NumberLine class.

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

To fix issue #2473 - default_numbers_to_display is currently being called by get_number_mobjects in NumberLine without being defined.

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

@zoya5636 zoya5636 changed the title Added missing default_numbers_to_display() in NumberClass Added missing default_numbers_to_display() in NumberLine Class Nov 29, 2024
@JasonGrace2282
Copy link
Member

Hello, thanks for your interest in contributing to Manim :)

When I try to test out the code, I get an error about op not existing, and fixing that I get an attribute error about there being no attribute tick_frequency on NumberLine.
For clarity, here's the test scene I used (straight from the issue mentioned)

class NumberlineNumberMobjects(Scene):
    def construct(self):
        line = NumberLine(include_numbers=True)
        numbers = line.get_number_mobjects()
        self.add(line)

I'll mark this PR as a draft until it's ready :)
Feel free to open it when you feel like it's working again.

@JasonGrace2282 JasonGrace2282 marked this pull request as draft December 7, 2024 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

2 participants