add api docs & examples for number_line module #1064

ksachdeva · 2021-02-27T21:49:47Z

Motivation

Adding the documentation & examples for number_line module

Overview / Explanation for Changes

added the apis
added the examples showing various ways NumberLine can be used

Oneline Summary of Changes

- API docs & examples for number_line module (:pr:`1064`)

Testing Status

Locally verified the generated document

Further Comments

Have not documented init_leftmost_tick as it should be considered a private method

Acknowledgements

[X ] I have read the Contributing Guidelines

Reviewer Checklist

Newly added functions/classes are either private or have a docstring
Newly added functions/classes have tests added and (optional) examples in the docs
Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
The oneline summary has been included in the wiki

jsonvillanueva

Thanks for the contribution! Wonderful example image illustrating the usage!

manim/mobject/number_line.py

Co-authored-by: Jason Villanueva <[email protected]>

manim/mobject/number_line.py

naveen521kk · 2021-03-01T19:52:02Z

manim/mobject/number_line.py

-        tick_frequency=1,
+        color: Colors = LIGHT_GREY,
+        unit_size: int = 1,
+        width: float = None,


use typing.Optional

naveen521kk · 2021-03-01T19:52:31Z

manim/mobject/number_line.py

-        x_max=config["frame_x_radius"],
+        numbers_with_elongated_ticks: typing.List[float] = [0],
+        include_numbers: bool = False,
+        numbers_to_show: typing.List[float] = None,


use typing.Optional

naveen521kk · 2021-03-01T19:53:04Z

manim/mobject/number_line.py

+        tip_height: float = 0.25,
+        add_start: float = 0,  # extend number line by this amount at its starting point
+        add_end: float = 0,  # extend number line by this amount at its end point
+        decimal_number_config: dict = {"num_decimal_places": 0},


Suggested change

decimal_number_config: dict = {"num_decimal_places": 0},

decimal_number_config: typing.Dict[str,int] = {"num_decimal_places": 0},

actually, it is not appropriate in this context because decimal_number_config is essentially the kwargs for DecimalNumber class and the values for the dictionary entries passed could other types.

Maybe Any instead of int is appropriate.

That said, I think this is too confusing and somewhere stems from the overuse of passing 'dictionaries' in manim.

The correct way should be to define a dataclass and then pass it around. ... nevertheless whatever we do writing scalable and maintainable code in python is always going to be tricky in this project or any other project.

Given all this ambiguity, my humble suggestion is to leave it at dict, and when (and if) you refactor the manim code to use proper types (instead of dictionaries) then the types would help otherwise this does not bring any value.

naveen521kk · 2021-03-01T19:53:46Z

manim/mobject/number_line.py

+        x_min: float = -config["frame_x_radius"],
+        x_max: float = config["frame_x_radius"],
        **kwargs
    ):


Suggested change

):

) -> None:

naveen521kk · 2021-03-01T19:54:04Z

manim/mobject/number_line.py

                self.tick_frequency, np.ceil(self.x_min / self.tick_frequency)
            )

    def add_tick_marks(self):


Suggested change

def add_tick_marks(self):

def add_tick_marks(self) -> None:

naveen521kk · 2021-03-01T19:54:35Z

manim/mobject/number_line.py

        )

-    def get_tick(self, x, size=None):
+    def get_tick(self, x: float, size: float = None) -> Line:


Suggested change

def get_tick(self, x: float, size: float = None) -> Line:

def get_tick(self, x: float, size: typing.Optional[float] = None) -> Line:

naveen521kk · 2021-03-01T19:56:30Z

manim/mobject/number_line.py

+        number_config: dict = None,
+        scale_val: float = None,
+        direction: np.ndarray = None,
+        buff: float = None,


Suggested change

number_config: dict = None,

scale_val: float = None,

direction: np.ndarray = None,

buff: float = None,

number_config: typing.Optional[dict] = None,

scale_val: typing.Optional[float] = None,

direction: typing.Optional[np.ndarray] = None,

buff: typing.Optional[float] = None,

naveen521kk · 2021-03-01T19:57:53Z

manim/mobject/number_line.py

+        tick_frequency: float = 0.1,
+        numbers_with_elongated_ticks: typing.List[float] = [0, 1],
+        number_at_center: float = 0.5,
+        decimal_number_config: dict = {


Suggested change

decimal_number_config: dict = {

decimal_number_config: typing.Dict[sstr,int] = {

I think u have a typo here and I am not sure it is correct typing annotation in the first place. See above comment.

ksachdeva · 2021-03-01T20:43:39Z

@naveen521kk I do not see the value in adding Optional typing where the None is specified (as it is obvious that argument is optional). I think it is overkill to do so and comes in the way of readability.

I am not going to add the typings that you are recommending for the reason mentioned above.

That said, I do respect the conventions and policies of a given project so if you want to archive/delete this pull request I am okay with it. Or if you want to merge it as it is and then do the necessary modifications as a follow-up as you see fit.

Thanks
Kapil

add api docs & examples for number_line module

d190299

jsonvillanueva approved these changes Feb 28, 2021

View reviewed changes

manim/mobject/number_line.py Show resolved Hide resolved

manim/mobject/number_line.py Outdated Show resolved Hide resolved

ksachdeva and others added 2 commits February 28, 2021 08:51

Update manim/mobject/number_line.py

cd58f20

Co-authored-by: Jason Villanueva <[email protected]>

Update manim/mobject/number_line.py

421051a

Co-authored-by: Jason Villanueva <[email protected]>

naveen521kk requested changes Feb 28, 2021

View reviewed changes

manim/mobject/number_line.py Outdated Show resolved Hide resolved

manim/mobject/number_line.py Show resolved Hide resolved

manim/mobject/number_line.py Show resolved Hide resolved

manim/mobject/number_line.py Show resolved Hide resolved

ksachdeva added 2 commits February 28, 2021 16:05

add typings to the classes & method

7f4839e

use numpy.ndarray instead of np.array

62c5926

naveen521kk requested changes Mar 1, 2021

View reviewed changes

ksachdeva closed this Mar 12, 2021

jsonvillanueva mentioned this pull request Mar 23, 2021

Moved previous version changelogs to separate files; Added a Script to generate future changelogs #1138

Merged

6 tasks

	decimal_number_config: dict = {"num_decimal_places": 0},
	decimal_number_config: typing.Dict[str,int] = {"num_decimal_places": 0},

	def get_tick(self, x: float, size: float = None) -> Line:
	def get_tick(self, x: float, size: typing.Optional[float] = None) -> Line:

	decimal_number_config: dict = {
	decimal_number_config: typing.Dict[sstr,int] = {

add api docs & examples for number_line module #1064

add api docs & examples for number_line module #1064

Uh oh!

Conversation

ksachdeva commented Feb 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Overview / Explanation for Changes

Oneline Summary of Changes

Testing Status

Further Comments

Acknowledgements

Reviewer Checklist

Uh oh!

jsonvillanueva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ksachdeva commented Mar 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ksachdeva commented Feb 27, 2021 •

edited

Loading