Converting the old docstrings to numpydoc style and hiding the license from rendering in every file

George-Elhamy commented 2 months ago

Description

These changes are mainly on converting the already found docstrings in src into numpydoc style which is more readable and better looking when rendered in sphinx and this has been done in two different commits. Along with that, the license have also been hidden from rendering (it still shows in the code in every rst file in the src & it also still shows in the documentation but only in the license section) and this has been done on two commits. There is also two python scripts included in the first commit that claculate the documentation coverage in the whole project so that we can track the coverage as we go on with adding the missing documentation for methods and classes.

Motivation and Context

How Has This Been Tested?

Screenshots (only if appropriate, delete if not):

Types of changes

[x] Bug fix
[ ] New feature
[ ] Breaking change (Causes existing functionality to not work as expected)

Checklist:

[x] My code follows the code style of this project.
[ ] I have documented my changes in /docs/changelog.md.
[ ] My change requires additional/updated documentation.
[x] I have updated the documentation accordingly.
[ ] I have added/updated the config example for any module docstrings as necessary.
[x] I have checked that the change does not contain obvious errors (syntax, indentation, mutable default values, etc.).
[ ] I have tested my changes using 'Load all modules' on the default dummy configuration.
[ ] All changed Jupyter notebooks have been stripped of their output cells.

timoML commented 2 months ago

To see the built version: https://qudi-core-testing.readthedocs.io/en/george/index.html

prithviulm commented 1 month ago

Sorry for again making numerous changes and for the force push. I rebased the PR with 2a3f75e41e47b4225abe9f3d6da8f8c2476ced87 and manually made the various fixes. This was a mess unfortunately. Hopefully the PR is easier to review now. I've tried to remove all the double quote changes and other formatting changes and other mistakes in the docstrings and licenses etc.

timoML commented 1 month ago

I think Sphinx files (Makefile, conf.py) should go into a seperate folder docs/sphinx
Did we want to keep string literals as they were or change to consistent use?

timoML commented 1 month ago

Runs on dummy with "load all modules". So indentation in (non-docstring) code seems fine.

Neverhorst commented 3 weeks ago

Is the double-dot .. in the license/copyright disclaimer needed? Seems weird to introduce this in all files since it is not required for a simple text body as far as I understand.

timoML commented 3 weeks ago

It's indicating to sphinx to not render the license for the documentation of every class. Is there another way for achieving the same?

Neverhorst commented 3 weeks ago

It's indicating to sphinx to not render the license for the documentation of every class. Is there another way for achieving the same?

Not from the top of my head but this documentation on docstring preprocessing seems promising. See also this post on stackoverflow as example.

prithviulm commented 3 weeks ago

It assumes that text at the top of the file is the module docstring and so the license also gets rendered along with the module docstring (if any). There might be some kind of workaround using regex to intercept the text and remove the license after Sphinx reads the text but before it starts parsing.

timoML commented 2 weeks ago

As discussed, we should revert .. in the license text to make this PR mergeable now, as it's a undocumented workaround. Most likely, we can later add some ignore escape sequence to Sphinx's conf.py, like:

"""(c)
This file contains ...
"""(c)

prithviulm commented 1 week ago

Ok, I think this is ready to merge now. Fixed a few more docstrings that were not converted. Reverted the license back to remove the leading ... As discussed, a style guide is necessary for things like when and where to use newlines in docstrings as Ruff will not handle these. I will add a brief description of it to the Ruff config.

prithviulm commented 1 week ago

Check that it still runs on dummy with "load all modules".

prithviulm commented 5 days ago

@Neverhorst We've addressed the remaining changes but can't merge till you approve.

Ulm-IQO / qudi-core