|
| 1 | +# Contributors |
| 2 | + |
1 | 3 | OpenNMT-py is a community developed project and we love developer contributions. |
2 | 4 |
|
| 5 | +## Guidelines |
3 | 6 | Before sending a PR, please do this checklist first: |
4 | 7 |
|
5 | | -- Please run `onmt/tests/pull_request_chk.sh` and fix any errors. When adding new functionality, also add tests to this script. Included checks: |
6 | | - 1. flake8 and pep8-naming check for coding style; |
| 8 | +- Please run `tools/pull_request_chk.sh` and fix any errors. When adding new functionality, also add tests to this script. Included checks: |
| 9 | + 1. flake8 check for coding style; |
7 | 10 | 2. unittest; |
8 | 11 | 3. continuous integration tests listed in `.travis.yml`. |
9 | | -- When adding/modifying class constructor, please make the arguments as same naming style as its superclass in pytorch. |
10 | | -- If your change is based on a paper, please include a clear comment and reference in the code. |
11 | | -- If your function takes/returns tensor arguments, please include assertions to document the sizes. See `GlobalAttention.py` for examples. |
| 12 | +- When adding/modifying class constructor, please make the arguments as same naming style as its superclass in PyTorch. |
| 13 | +- If your change is based on a paper, please include a clear comment and reference in the code (more on that below). |
| 14 | + |
| 15 | +### Docstrings |
| 16 | +Above all, try to follow the Google docstring format |
| 17 | +([Napoleon example](https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html), |
| 18 | +[Google styleguide](http://google.github.io/styleguide/pyguide.html)). |
| 19 | +This makes it easy to include your contributions in the Sphinx documentation. And, do feel free |
| 20 | +to autodoc your contributions in the API ``.rst`` files in the `docs/source` folder! If you do, check that |
| 21 | +your additions look right. |
| 22 | + |
| 23 | +```bash |
| 24 | +cd docs |
| 25 | +# install some dependencies if necessary: |
| 26 | +# recommonmark, sphinx_rtd_theme, sphinxcontrib-bibtex |
| 27 | +make html |
| 28 | +firefox build/html/main.html # or your browser of choice |
| 29 | +``` |
| 30 | + |
| 31 | +Some particular advice: |
| 32 | +- Try to follow Python 3 [``typing`` module](https://docs.python.org/3/library/typing.html) conventions when documenting types. |
| 33 | + - Exception: use "or" instead of unions for more readability |
| 34 | + - For external types, use the full "import name". Common abbreviations (e.g. ``np``) are acceptable. |
| 35 | + For ``torch.Tensor`` types, the ``torch.`` is optional. |
| 36 | + - Please don't use tics like `` (`str`) `` or rst directives like `` (:obj:`str`) ``. Napoleon handles types |
| 37 | + very well without additional help, so avoid the clutter. |
| 38 | +- [Google docstrings don't support multiple returns](https://stackoverflow.com/questions/29221551/can-sphinx-napoleon-document-function-returning-multiple-arguments). |
| 39 | +For multiple returns, the following works well with Sphinx and is still very readable. |
| 40 | + ```python |
| 41 | + def foo(a, b): |
| 42 | + """This is my docstring. |
| 43 | + |
| 44 | + Args: |
| 45 | + a (object): Something. |
| 46 | + b (class): Another thing. |
| 47 | + |
| 48 | + Returns: |
| 49 | + (object, class): |
| 50 | + |
| 51 | + * a: Something or rather with a long |
| 52 | + description that spills over. |
| 53 | + * b: And another thing. |
| 54 | + """ |
| 55 | + |
| 56 | + return a, b |
| 57 | + ``` |
| 58 | +- When citing a paper, avoid directly linking in the docstring! Add a Bibtex entry to `docs/source/refs.bib`. |
| 59 | +E.g., to cite "Attention Is All You Need", visit [arXiv](https://arxiv.org/abs/1706.03762), choose the |
| 60 | +[bibtext](https://dblp.uni-trier.de/rec/bibtex/journals/corr/VaswaniSPUJGKP17) link, search `docs/source/refs.bib` |
| 61 | +using `CTRL-F` for `DBLP:journals/corr/VaswaniSPUJGKP17`, and if you do not find it then copy-paste the |
| 62 | +citation into `refs.bib`. Then, in your docstring, use ``:cite:`DBLP:journals/corr/VaswaniSPUJGKP17` ``. |
| 63 | + - However, a link is better than nothing. |
| 64 | +- Please document tensor shapes. Prefer the format |
| 65 | + ``` ``(a, b, c)`` ```. This style is easy to read, allows using ``x`` for multplication, and is common |
| 66 | + (PyTorch uses a few variations on the parentheses format, AllenNLP uses exactly this format, Fairseq uses |
| 67 | + the parentheses format with single ticks). |
| 68 | + - Again, a different style is better than no shape documentation. |
| 69 | +- Please avoid unnecessary space characters, try to capitalize, and try to punctuate. |
| 70 | + |
| 71 | + For multi-line docstrings, add a blank line after the closing ``"""``. |
| 72 | + Don't use a blank line before the closing quotes. |
| 73 | + |
| 74 | + ``""" not this """`` ``"""This."""`` |
| 75 | + |
| 76 | + ```python |
| 77 | + """ |
| 78 | + Not this. |
| 79 | + """ |
| 80 | + ``` |
| 81 | + ```python |
| 82 | + """This.""" |
| 83 | + ``` |
| 84 | + |
| 85 | + This note is the least important. Focus on content first, but remember that consistent docs look good. |
| 86 | +- Be sensible about the first line. Generally, one stand-alone summary line (per the Google guidelines) is good. |
| 87 | + Sometimes, it's better to cut directly to the args or an extended description. It's always acceptable to have a |
| 88 | + "trailing" citation. |
0 commit comments