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 DOC string for all the geometric objects. #1024

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added DOC string for all the geometric objects. #1024

wants to merge 4 commits into from

Conversation

AkashKarnatak
Copy link

Added DOC string for all the geometric objects.
Earlier only ndarray could be used as arguments for ArcBetweenPoints instance.
For eg:-

arc = ArcBetweenPoints(start = (1,2,0), end = (-2,-3,0), angle = 30 * DEGREES)

would give error. But after the changes it works fine.

Thanks for contributing to manim!

Please ensure that your pull request works with the latest version of manim.
You should also include:

  1. The motivation for making this change (or link the relevant issues)
  2. How you tested the new behavior (e.g. a minimal working example, before/after
    screenshots, gifs, commands, etc.) This is rather informal at the moment, but
    the goal is to show us how you know the pull request works as intended.

Earlier only ndarray could be used as arguments for some
geometric objects, for eg:- starting and ending point of
Line object. Now any array_like object can be used.
because there arguments were already compatible with array_like
objects.
Copy link

@kevkevinpal kevkevinpal left a comment

Choose a reason for hiding this comment

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

Other than maybe changing requirements -> parameters great job and thank you for the documentation really helps a ton!

Returns
-----
out : Arc object
An Arc object satisfying the specified requirements

Choose a reason for hiding this comment

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

maybe requirements -> parameters so it can match whats above same thing for the other blocks of comments

Copy link
Author

Choose a reason for hiding this comment

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

Should I make a new pull request changing requirements to parameters?

Choose a reason for hiding this comment

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

Nah just making a commit and pushing to the existing branch should be sufficient

Copy link
Author

Choose a reason for hiding this comment

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

ok

Copy link

@kevkevinpal kevkevinpal left a comment

Choose a reason for hiding this comment

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

Looks good nice job 👌

anooprh pushed a commit to anooprh/manim that referenced this pull request Feb 14, 2021
* Migrate width/height/depth to properties

* Fix examples

* Fix typos

* Import manim for doctests

* Expect Square for doctests

* Improve docs for width/height/depth properties
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.

None yet

2 participants