Add documentation for unit.py unit conversion feature

[tameTNT]

Overview: What does this pull request change?

• Adds documentation and 2 examples for unit helper classes Percent, Pixels and Degrees
• (Hopefully) Fixes bug in class.rst where Methods header is displayed even when there are no public methods to list.

Motivation and Explanation: Why and how do your changes improve the library?

• Closes #2512

Links to added or changed documentation pages

• https://manimce--2604.org.readthedocs.build/en/2604/reference_index/utilities_misc.html
• https://manimce--2604.org.readthedocs.build/en/2604/reference/manim.utils.unit.html#module-manim.utils.unit
• https://manimce--2604.org.readthedocs.build/en/2604/reference/manim.utils.unit.Percent.html#manim.utils.unit.Percent
• https://docs.manim.community/en/stable/reference/manim.utils.tex.TexTemplate.html#:~:text=str-,Methods,-add_to_document (example of potentially affected methods header in current docs)
• https://manimce--2604.org.readthedocs.build/en/2604/reference/manim.utils.tex.TexTemplate.html#:~:text=str-,Methods (with proposed changes - hopefully no different)

Further Information and Comments

• This is my first contribution to the repository! I tried to follow all the style guides and documentation formats given by the contributing wiki but I've inevitably missed something I'm sure. Just let me know where I've messed up and I'll try and fix it right away.
• I have a habit of making lots of small commits instead of fewer larger ones so please squash and merge this PR instead of just merging it for everyone's sake!

Reviewer Checklist

• [ ] The PR title is descriptive enough for the changelog, and the PR is labeled correctly
• [ ] If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
• [ ] If applicable: newly added functions and classes are tested

[tameTNT]

And I've found my first mistake - I didn't actually update the docs. Apologies if this is an incredibly stupid question but how do I actually do that? Do I just add ~utils.unit to docs/source/reference_index/utilities_misc.rst? Should I update a changelog too or will that be done for me?

[tameTNT]

Do I just add ~utils.unit to docs/source/reference_index/utilities_misc.rst?

Okay so I tried this and it worked. 🥳

• However, I don't know how to get the example/code snippet for Pixels and Degrees to be rendered in the docs too since these are 'just' variables and not classes/functions.
• Also, is there a way to remove the trailing Methods header on the Percent wiki page since this isn't really applicable?

[tameTNT]

That's great - thank you so much for the reviews! I'll have a look at addressing them properly when I have time over the next few days. 👍

[tameTNT]

Just to mark that this PR isn't dead - I will continue with this soon! Unfortunately some personal issues came up over the last week that meant I was unable to do the further work I would have liked.

[tameTNT]

This might be caused by the template in docs/source/_templates/autosummary/class.rst checking for the "wrong" thing on line 12.

@behackl

I think this might due to the fact that Percent has special members (in this case also inherited_members) of __mul__ and __rmul__ so {%- if methods %} evaluates to True, generating the methods header - .. rubric:: {{ _('Methods') }} - but then for item in methods if item != '__init__' and item not in inherited_members is an empty list, so no methods are actually added beneath the header in the loop.

~~I believe the above change to class.rst might fix it but my Jinga's not the best 😂~~

    {% set methods_to_list = [item for item in methods if item != '__init__' and item not in inherited_members] %}
jinja2.exceptions.TemplateSyntaxError: expected token ',', got 'for'

Jinga doesn't support list comprehensions... Working on an alternative solution

[tameTNT]

Currently looking into something with filters (https://jinja.palletsprojects.com/en/3.0.x/templates/#list-of-builtin-filters):

{%- if methods|select('ne', '__init__')|select('in', inherited_members) %} for line 12 in class.rst.

But we actually need notin since we're trying to match the list generated in {% for item in methods if item != '__init__' and item not in inherited_members %}.

For now, I don't think I'll be able to do any more of this today but I'll have another look tomorrow. Any suggestions in the meantime are greatly appreciated!

[tameTNT]

I've exhausted my options I think at this point. I'm going to open this PR up for review with the hope that someone will know how to address the bug (for initial description see the earlier comment).

To aid them, here's a summary of all I've tried (sometimes stupidly) in the commits above:

1. List comprehension

   {% set methods_to_list = [item for item in methods if item != '__init__' and item not in inherited_members] %}
   

2. Jinja filters

   {% set no_init_m = methods | select('ne', '__init__') | set %}
   {%- if no_init_m.difference(set(inherited_members)) %}
   ...
   {% for item in methods if item != '__init__' and item not in inherited_members %}
   

3. Set operations

   {% set no_init_m = methods | select('ne', '__init__') %}
   {%- if set(no_init_m).difference(set(inherited_members)) %}
   

4. Implied list comprehension

   {% set methods_to_list = methods if item != '__init__' and item not in inherited_members %}
   

5. Split implied list comprehension

   {% set methods_to_list = methods if item not in inherited_members %}
   {% set methods_to_list = methods_to_list if item != '__init__' %}
   

6. Explicit list building

   {% set displayed_methods = [] %}
   {% for item in methods %}
      {% if item != '__init__' and item not in inherited_members %}
         {% do displayed_methods.append(item) %}
      {%- endif %}
   {%- endfor %}
   

7. Same as above but with following instead of do clause

   {% set displayed_methods = displayed_methods + [item] %}
   

8. Nested if statements

   {%- for m in methods %}
      {%- if m != '__init__' %}
         {%- if m not in inherited_members %}
            {% set displayed_methods = displayed_methods + [m] %}
         {%- endif %}
      {%- endif %}
   {%- endfor %}
   

I hope someone will be able to narrow down what I haven't covered yet. Currently, no methods nor any Methods header is displayed at all on any pages (check the links in the starter comment to compare stable docs with PR docs).

[kilacoda]

Hi, just checking up on this. Is it ready for review? You've marked it as a draft and have requested a review from @behackl so that's why it seems a bit confusing. The changes seem fine to me tbh so if you think this is ready for review I'd be up for reviewing and merging it soon.

[tameTNT]

Hi! Yeah so as my last comment summarises that I'd hit a snag when it came to editing the documentation template that I wasn't sure how to fix. It outlines what I've already tried to fix the bug so I'd marked the PR as a draft until someone found a solution (it's probably simple I just don't know Jenja well enough 😂). But in terms of the actual documentation this PR is complete. 👍

[tameTNT]

Hi! Thank you for somehow finding this old relic of a PR 😆 and for contributing to the project 🤩.

I've just taken some time to refamiliarise myself with what I was even doing here and some observations:

• The removal of the basic Pixels example as suggested by @JasonGrace2282 is fair I think, but maybe a small example line should be included in the (as with Unit.Degrees) so there is at least some sample usage even if there isn't a full video.
• I can see the work you've done in your PR and it looks cool! As you say, as you've clearly reworked a large part of this part of the docs rendering process, best to wait until your PR is merged to transition the three module attribute docs (Pixels, Degrees, MUnits) for this PR to be rendered in the docs using your new tools (and then put the examples in line instead of under separate headings as was compromised initially here).
• The methods heading changes I've made (in docs/source/_templates/autosummary/class.rst) should be rolled back before any merging with this branch takes place as these changes currently break existing documentation pages (e.g. Methods header missing on TexTemplate page on this PR's build (c.f. stable docs)). Once your PR is merged then this can be reattempted of course in a separate PR is probably best with a more appropriate name, etc.

[JasonGrace2282]

To be clear, I'm not against keeping an example with the different units just removing the reST that says unit.Pixels Example

[JasonGrace2282]

If you're still interested, Chopan's PR has now been merged.

[tameTNT]

To be completely honest, I don't really have the time at the moment nor the familiarity with the project/repo as a whole anymore since it has been so long since I worked on it, so I don't think I'd be able to do a satisfactory job.

[JasonGrace2282]

That's completely fine, life comes first! I'll just close this PR and create an issue to remind ourselves about this.