Converted all types of parameters in docstrings to proper type annotations #2988
Conversation
NotWearingPants
force-pushed
the
cleanup/docstring-types
branch
from
952894b to
03618d2
Compare
|
The first commit was created by running this script I wrote, so if you prefer you can review the script instead of the diff it created. For the 2nd commit I ran |
behackl
added
the
maintenance
There was a problem hiding this comment.
Very impressive work, kudos! I have a bunch of minor suggestions / change requests, but am overall very happy with this. This is indeed a substantial step in the direction of a fully typed library! 🥳
Please let me know if you can take a look at my comments, otherwise I'll work on resolving them in order to get this PR merged ASAP. Thanks!
| The new time the animation should take in seconds. | ||
|
|
||
| rate_func | ||
| The new function defining the animation progress based on the relative runtime (see :mod:`~.rate_functions`). |
There was a problem hiding this comment.
line length
| The new function defining the animation progress based on the relative runtime (see :mod:`~.rate_functions`). | |
| The new function defining the animation progress based | |
| on the relative runtime (see :mod:`~.rate_functions`). |
| pixel_width: int | None = None, | ||
| frame_height: float | None = None, | ||
| frame_width: float | None = None, | ||
| frame_rate: float | None = None, | ||
| **kwargs, | ||
| ): | ||
| """Initialises the Camera. |
There was a problem hiding this comment.
The docstring of Camera.__init__ should actually live in the class docstring of Camera (otherwise it won't show up in the built documentation).
| background : optional | ||
| background_image | ||
| The path to an image that should be the background image. | ||
| If not set, the background is filled with `self.background_color` |
There was a problem hiding this comment.
| If not set, the background is filled with `self.background_color` | |
| If not set, the background is filled with :attr:`background_color` |
or
| If not set, the background is filled with `self.background_color` | |
| If not set, the background is filled with ``self.background_color`` |
(I'd prefer the first.)
| An integer constant described in the Pillow library, | ||
| or one from the RESAMPLING_ALGORITHMS global dictionary, | ||
| under the following keys: | ||
| * 'bicubic' or 'cubic' |
There was a problem hiding this comment.
See https://manimce--2988.org.readthedocs.build/en/2988/reference/manim.mobject.types.image_mobject.AbstractImageMobject.html#manim.mobject.types.image_mobject.AbstractImageMobject.set_resampling_algorithm -- if this should be formatted as a list, there has to be a newline between lines 69 and 70.
| animations | ||
| Animation or mobject with mobject method and params | ||
| play_kwargs | ||
| named parameters affecting what was passed in ``animations``, |
There was a problem hiding this comment.
| named parameters affecting what was passed in ``animations``, | |
| Named parameters affecting what was passed in ``animations``, |
There was a problem hiding this comment.
I could not push my changes here directly, so I have made a PR to your fork: NotWearingPants#1 -- please take a look, I'd like to get this merged rather sooner than later. :-)
|
@behackl I'm sorry I wasn't responsive this month so far, I've been busy with school work unfortunately (semester started recently). |
behackl
changed the title
Overview: What does this pull request change?
I have gone through all of the types of parameters in docstrings and:
Motivation and Explanation: Why and how do your changes improve the library?
Advances progress in #2121 by a lot 🎉
There are still return types and attribute types in docstrings, maybe I'll do them next.
Links to added or changed documentation pages
Further Information and Comments
I wrote a script to help me automate a lot of the process, and manually did what I couldn't automate (the 4th commit).
Reviewer Checklist