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

Start to cleanup PyOutline and PyCue docstrings #589

Merged
merged 2 commits into from
Dec 23, 2019
Merged

Start to cleanup PyOutline and PyCue docstrings #589

merged 2 commits into from
Dec 23, 2019

Conversation

sharifsalah
Copy link
Collaborator

@sharifsalah sharifsalah commented Dec 20, 2019

Link the Issue(s) this Pull Request is related to.
Related to #559

Summarize your change.
Update PyOutline and PyCue docstrings to resolve warnings when building the API reference. Also improve some of the formatting to call out parameters and return types. I also fixed some inaccurate docstrings as I came across them and improved formatting of some of the Python example code in the docstrings. There's more to do, including updating types so that they link between the generated docs and to improve coverage, clarity, and accuracy of the content. I made the changes using a mixture of regex search and replace and some manually editing.

Mostly, the changes are to update docstrings in the default expected by Sphinx, from:

       """Return an existing limit by it's id.
        @type id: str
        @param id: Name of limit to find.
        @rtype: opencue.wrappers.limit.Limit
        @return: The limit found by id.
        """

To the following:

        """Return an existing limit by it's id.
        
        :type id: str
        :param id: Name of limit to find.
        :rtype: opencue.wrappers.limit.Limit
        :return: The limit found by id.
        """

The output has been updated from something like the following:

Screen Shot 2019-12-20 at 8 49 22 AM

To the following updated output:

Screen Shot 2019-12-20 at 3 47 42 PM

One file to pay particular attention to is pycue/opencue/wrappers/limit.py. The indentation throughout this file was different from the others and was causing Sphinx to throw an exception when I added a description of the class.

The build output is now clean with no warnings or errors:

$ make html
Running Sphinx v1.8.5
making output directory...
loading intersphinx inventory from https://docs.python.org/objects.inv...
intersphinx inventory has moved: https://docs.python.org/objects.inv -> https://docs.python.org/3/objects.inv
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 11 source files that are out of date
updating environment: 11 added, 0 changed, 0 removed
reading sources... [100%] modules/outline.versions                                                                            
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] modules/outline.versions                                                                             
generating indices... genindex py-modindex
writing additional pages... search
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.

The HTML pages are in _build/html.

@sharifsalah sharifsalah marked this pull request as ready for review December 20, 2019 15:51
Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

This is approved with a few minor comments. Thanks for working on this @sharifsalah, it looks like a lot of work. The docs site looks great, and this will be a big help while doing dev work -- both the reference site itself as well as the improved IDE functionality that we get for free with these cleaned-up docstrings :)

docs/conf.py Show resolved Hide resolved
pycue/opencue/api.py Show resolved Hide resolved
pycue/opencue/wrappers/limit.py Outdated Show resolved Hide resolved
@sharifsalah
Copy link
Collaborator Author

Thanks for the review @bcipriano. Also very glad to hear that it improves the IDE experience for free :)

@sharifsalah sharifsalah merged commit eaef93d into AcademySoftwareFoundation:master Dec 23, 2019
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.

2 participants