Skip to content

Improve/extend reference manual #438

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

Merged
merged 24 commits into from
Sep 16, 2020
Merged

Conversation

behackl
Copy link
Member

@behackl behackl commented Sep 13, 2020

List of Changes

  • Add all modules to the reference manual.
  • Change class render template: class methods now don't get their own files any longer, their documentation is now located in their corresponding parent classes. (Also speeds up docbuild on RTD, from ~300sec to ~270sec.)
  • Fix all Sphinx warnings

TODO:

  • Add more documentation 🙃 (at least some more one-line module / class documentations...) -- help welcome!

Motivation

The reference manual should be as complete as possible, even if there is little to no documentation for the methods/objects.

Testing Status

See https://behackl-manim.readthedocs.io/en/improve-refman/reference.html for a deployed version.

Acknowledgement

@behackl behackl added documentation Improvements or additions to documentation enhancement Additions and improvements in general help wanted We would appreciate help on this issue/PR labels Sep 13, 2020
@behackl
Copy link
Member Author

behackl commented Sep 13, 2020

In a157c04, I tried adding summary docstrings for most submodules of mobject.

There are some modules where I am not sure what they are actually used for, and some where I did not really know what to write. The missing ones are:

  • mobject.changing
  • mobject.frame
  • mobject.mobject_update_utils
  • mobject.three_d_shading_utils
  • mobject.three_d_utils

Furthermore, when skimming through the modules, I felt that there are some inconsistencies (or at least weird choices), e.g., I feel that mobject.shape_matchers and mobject.brace could merge into mobject.annotation or so; mobject.value_tracker and mobject.mobject_update_utils seem compatible to some degree (if I understood their intended use correctly); mobject.three_d_utils and mobject.three_d_shading_utils seem compatible; and I also see some common ground for mobject.number_line and mobject.coordinate_systems. (I certainly don't propose changing all of this now and here, but I was curious in how far structural changes like these have been discussed before.)

See either the changed files here or https://behackl-manim.readthedocs.io/en/improve-refman/reference.html for a list of the others.

I suggest adding all of the remaining missing module summaries, and doing more detailed documentation in follow-up PRs (in order to not blow this out of proportion and keep it reviewable).

@leotrs
Copy link
Contributor

leotrs commented Sep 13, 2020

There are some modules where I am not sure what they are actually used for, and some where I did not really know what to write. The missing ones are:
Furthermore, when skimming through the modules, I felt that there are some inconsistencies (or at least weird choices), e.g., I feel that mobject.shape_matchers and mobject.brace could merge into mobject.annotation or so; mobject.value_tracker and mobject.mobject_update_utils seem compatible to some degree (if I understood their intended use correctly); mobject.three_d_utils and mobject.three_d_shading_utils seem compatible; and I also see some common ground for mobject.number_line and mobject.coordinate_systems. (I certainly don't propose changing all of this now and here, but I was curious in how far structural changes like these have been discussed before.)

There are numerous inconsistencies. I think what you've done is a big step forward, so I won't worry about those other modules just yet. Moreover, the fact that a dev is confused about them means that they're not ready for public documentation anyway.

I suggest adding all of the remaining missing module summaries, and doing more detailed documentation in follow-up PRs (in order to not blow this out of proportion and keep it reviewable).

Agreed.

Co-authored-by: Leo Torres <dleonardotn@gmail.com>
@behackl
Copy link
Member Author

behackl commented Sep 13, 2020

Alright, then lets do it like that! 👍

Thanks for the suggested changes; I first thought of writing more than just the one-line summary---but then decided against it; its better to have well-documented classes (and maybe even an improved structure for the modules) first.

@behackl
Copy link
Member Author

behackl commented Sep 14, 2020

manim.scene.reconfigureable_scene only contains the class ReconfigureableScene, which mentions prominently in its documentation that "Note, this seems to no longer work as intended."

I won't add a module summary to it; it should probably be removed.

@behackl
Copy link
Member Author

behackl commented Sep 16, 2020

This is complete from my point of view. While the diff looks a bit scary at first glance, there are three files that actually needed a bit more attention: mobject/svg/code_mobject.py, mobject/svg/text_mobject.py and utils/tex.py. For these, having a look at the rendered documentation might be easier than reading the diffs -- but unfortunately, something weird is happening over at RTD with this branch since I resolved the merge conflict such that it does not build the correct version; so building docs and looking at them has to be done locally.

@behackl behackl marked this pull request as ready for review September 16, 2020 01:43
@behackl behackl changed the title [WIP] Improve/extend reference manual Improve/extend reference manual Sep 16, 2020
Copy link
Member

@huguesdevimeux huguesdevimeux left a comment

Choose a reason for hiding this comment

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

SGTM !

"""Display Text.

Text objects behave like a :class:`.VGroup`-like iterable of all characters
in the given text. In particular, slicing is possible.
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion : (if-minor) I think we should specify that this does not use LaTex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good catch! 1aab895

behackl and others added 2 commits September 16, 2020 13:58
Co-authored-by: Hugues Devimeux <36239975+huguesdevimeux@users.noreply.github.com>
leotrs
leotrs previously requested changes Sep 16, 2020
behackl and others added 2 commits September 16, 2020 16:44
Co-authored-by: Leo Torres <dleonardotn@gmail.com>
@behackl
Copy link
Member Author

behackl commented Sep 16, 2020

All the open discussions should now be resolved with 356d111. :-)

@leotrs leotrs merged commit 5467978 into ManimCommunity:master Sep 16, 2020
@behackl behackl deleted the improve-refman branch May 16, 2022 09:15
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 enhancement Additions and improvements in general help wanted We would appreciate help on this issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants