googlegenomics / gcp-variant-transforms

GCP Variant Transforms
Apache License 2.0
134 stars 55 forks source link

Setup python docstring generation and correct existing type hints #108

Open arostamianfar opened 6 years ago

arostamianfar commented 6 years ago

We currently use python type hints in docstrings (e.g. arg_name (int): some argument). For class types, we use ``Class`` or sometimes :class:`Class`. This is to make sphinx understand the types and is a convention used in Beam as well.

We should first setup python doc generator using Sphinx (see Beam's implementation) and fix all broken type references.

bashir2 commented 6 years ago

I like to extend the scope of this issue to setup something that can actually do some sort of static type checking. Until we can use Python 3, it seems PEP 484 has a suggestion for Python 2.7 that we can use: Suggested syntax for Python 2.7 and straddling code I have tried this annotation and PyCharm understands/validates it but I have only used it in IDE. We should setup an automated tool that understands and validates these hints and add it as part of our presubmit checks.

bashir2 commented 6 years ago

I also checked with python-users@ and it seems the comment style suggestion or PEP-484 conforms to the style guide. It is also concise and makes the eventual move to Python 3 type annotation easy, so I am going to send PRs to update the code with this and setup a presubmit check.

arostamianfar commented 6 years ago

Thanks, Bashir! I also really like the comment style type definitions! I now wonder if it makes sense to just use PEP-484 and remove types from our Args completely. Do you know if there is a doc generator that understands the comment style docs? I'm thinking of having API docs similar to Beam where you can click on types in the comments, but I find the sphinx style confusing (e.g. the :class: operators) and very easy to get wrong (e.g. most of the links in the Beam doc don't work).

P.S. I came across the "Google style" args checker (it was kinda noisy when I tried it though). It also doesn't enforce types.

bashir2 commented 6 years ago

Asha has created separate issues for type checking, namely Issue #189, #190, so I am unassigning myself since type checking was the main part I was planning to do. I may pick #190 later on in the quarter if I find any free cycles.

arostamianfar commented 6 years ago

I managed to create the docs using Sphinx (similar to what Beam does), but I actually don't think that it's super useful at this point and we'd need to somehow have an automatic way of updating the docs as well. So, lowering the priority for now.

If anyone wants to generate the docs themselves, they can just run the following from the root of gcp-variant-transforms directory. The docs will be under docs/api/_build/index.html.

pip install sphinx sphinx_rtd_theme
python $(type -p sphinx-apidoc) -fMeT -o docs/api gcp_variant_transforms
cat > docs/api/conf.py <<'EOF'
import os
import sys
import sphinx_rtd_theme
extensions = [
    'sphinx.ext.autodoc',
    'sphinx.ext.doctest',
    'sphinx.ext.intersphinx',
    'sphinx.ext.napoleon',
    'sphinx.ext.viewcode',
]
master_doc = 'index'
html_theme = 'sphinx_rtd_theme'
html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
project = 'GCP Variant Transforms'
autoclass_content = 'both'
autodoc_member_order = 'bysource'
EOF
cat > docs/api/index.rst <<'EOF'
.. include:: ./gcp_variant_transforms.rst
   :start-line: 2
EOF
python $(type -p sphinx-build) -v -a -E -j 8 -q docs/api docs/api/_build -c docs/api