Skip to content

Conversation

@kolibril13
Copy link
Member

@kolibril13 kolibril13 commented Mar 31, 2021

As there are still a lot of undocumented functions, this pr will add some more docstings to mobject methods (e.g. rotate, get_x, flip, etc.
EDIT: SEE rendered page here: https://manimce--1206.org.readthedocs.build/en/1206/reference/manim.mobject.mobject.Mobject.html

Checklist

  • I have read the Contributing Guidelines
  • I have written a descriptive PR title (see top of PR template for examples)
  • I have written a changelog entry for the PR or deem it unnecessary
  • My new functions/classes either have a docstring or are private
  • My new functions/classes have tests added and (optional) examples in the docs
  • My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

  • The PR title is descriptive enough
  • The PR is labeled correctly
  • The changelog entry is completed if necessary
  • Newly added functions/classes either have a docstring or are private
  • 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

@kolibril13
Copy link
Member Author

I am just wondering:
https://docs.manim.community/en/latest/examples.html#rotationupdater
has a reference to add_updater()but it is not possible to click on it, however there is this page that gives an example how to use updater:
https://docs.manim.community/en/latest/reference/manim.mobject.mobject.Mobject.html#manim.mobject.mobject.Mobject.add_updater

@jsonvillanueva
Copy link
Member

jsonvillanueva commented Mar 31, 2021

I figured out the issue, it's because our manim_directive.py has :ref_functions: link to :func:~.{func_here}... but the issue with this is that these aren't functions. They are methods. As such we need a :ref_methods: to link to :meth:~.{meth_here}

@kolibril13
Copy link
Member Author

"ref_functions": lambda arg: process_name_list(arg, "func"),

Found it!
ok, I can add this to this pr.

@jsonvillanueva jsonvillanueva added the documentation Improvements or additions to documentation label Mar 31, 2021
@jsonvillanueva jsonvillanueva changed the title Added Docstings to Mobject Added Docstrings to :class:~.Mobject Mar 31, 2021
@kolibril13
Copy link
Member Author

Ready for review!

Copy link
Member

@jsonvillanueva jsonvillanueva left a comment

Choose a reason for hiding this comment

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

Mostly small things. Periods at the ends of sentences, some references to :class:`~.Mobject`.

Also, some references to :meth:`~.Mobject.xxxxxxxx` should be used instead of ``xxxxxxxx``. Technically this documentation should have type hints as well as we've been enforcing that.

@kolibril13
Copy link
Member Author

Thanks for the review!
Should there really be a reference to all mobjects?
I think is not really necessary, as this is the page of mobject.
Also, it is also not there in already existing docstrings of other methods in mobject:
image
However, if this is the better approach, I am fine with doing it that way.

return self.match_coord(mobject, 2, direction)

def align_to(self, mobject_or_point, direction=ORIGIN, alignment_vect=UP):
def align_to(self, mobject_or_point:Union["Mobject",np.ndarray, List], direction=ORIGIN, alignment_vect=UP):
Copy link
Member Author

Choose a reason for hiding this comment

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

As I am using type hints the first time, I am not sure if this is the best way to add them in this way Union["Mobject",np.ndarray, List], or if it is even possible to specify that the list/array needs to have exactly 3 entries.

@jsonvillanueva
Copy link
Member

Thanks for the review!
Should there really be a reference to all mobjects?
I think is not really necessary, as this is the page of mobject.
Also, it is also not there in already existing docstrings of other methods in mobject:
image
However, if this is the better approach, I am fine with doing it that way.

This is more about completeness, but essentially these links will take the reader back to the top of the page that they're already on. Similarly, if you mention a method/function, they should also be linked to properly (even if they're on the same page) because the user can quickly jump to that section if they want to click on it and go to it.

@jsonvillanueva jsonvillanueva merged commit f3bd42f into ManimCommunity:master Apr 2, 2021
@kolibril13 kolibril13 deleted the added_docstings_mobject branch April 2, 2021 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants