Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Commit

Permalink
improve contributing guide / PR template (#5185)
Browse files Browse the repository at this point in the history
* improve contributing guide / PR template

* update coverage config

* update template
  • Loading branch information
epwalsh authored May 7, 2021
1 parent 7a260da commit b1b455a
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 5 deletions.
24 changes: 20 additions & 4 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
<!-- Please reference the issue number here -->
<!-- To ensure we can review your pull request promptly please complete this template entirely. -->

<!-- Please reference the issue number here. You can replace "Fixes" with "Closes" if it makes more sense. -->
Fixes # .

Changes proposed in this pull request:

-
<!-- Please list all changes/additions here. -->
-

<!-- To ensure we can review your pull request promptly please ensure ALL GitHub Actions workflows pass -->
## Before submitting

<!-- Please complete this checklist BEFORE submitting your PR to speed along the review process. -->
- [ ] I've read and followed all steps in the [Making a pull request](https://github.com/allenai/allennlp/blob/main/CONTRIBUTING.md#making-a-pull-request)
section of the `CONTRIBUTING` docs.
- [ ] I've updated or added any relevant docstrings following the syntax described in the
[Writing docstrings](https://github.com/allenai/allennlp/blob/main/CONTRIBUTING.md#writing-docstrings) section of the `CONTRIBUTING` docs.
- [ ] If this PR fixes a bug, I've added a test that will fail without my fix.
- [ ] If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

## After submitting

<!-- Please complete this checklist AFTER submitting your PR to speed along the review process. -->
- [ ] All GitHub Actions jobs for my pull request have passed.
- [ ] **`codecov/patch`** reports high test coverage (at least 90%).
You can find this under the "Actions" tab of the pull request once the other checks have finished.
98 changes: 97 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ When you're ready to contribute code to address an open issue, please follow the

pytest -v --cov allennlp.nn.util tests/nn/util_test.py

If your contribution involves changes to any docstrings, you should make sure the API documentation can build without errors. For that, just run
If your contribution involves additions to any public part of the API, we require that you write docstrings
for each function, method, class, or module that you add.
See the [Writing docstrings](#writing-docstrings) section below for details on the syntax.
You should test to make sure the API documentation can build without errors by running

make build-docs

Expand All @@ -159,6 +162,99 @@ When you're ready to contribute code to address an open issue, please follow the

</details>

### Writing docstrings

Our docstrings are written in a syntax that is essentially just Markdown with additional special syntax for writing parameter descriptions.

Class docstrings should start with a description of the class, followed by a `# Parameters` section
that lists the names, types, and purpose of all parameters to the class's `__init__()` method.
Parameter descriptions should look like:

```
name : `type`
Description of the parameter, indented by four spaces.
```

Optional parameters can also be written like this:

```
name : `type`, optional (default = `default_value`)
Description of the parameter, indented by four spaces.
```

Sometimes you can omit the description if the parameter is self-explanatory.

Method and function docstrings are similar, but should also include a `# Returns`
section when the return value is not obvious. Other valid sections are

- `# Attributes`, for listing class attributes. These should be formatted in the same
way as parameters.
- `# Raises`, for listing any errors that the function or method might intentionally raise.
- `# Examples`, where you can include code snippets.

Here is an example of what the docstrings should look like in a class:


```python
class SentenceClassifier(Model):
"""
A model for classifying sentences.
This is based on [this paper](link-to-paper). The input is a sentence and
the output is a score for each target label.
# Parameters
vocab : `Vocabulary`
text_field_embedder : `TextFieldEmbedder`
The text field embedder that will be used to create a representation of the
source tokens.
seq2vec_encoder : `Seq2VeqEncoder`
This encoder will take the embeddings from the `text_field_embedder` and
encode them into a vector which represents the un-normalized scores
for the target labels.
dropout : `Optional[float]`, optional (default = `None`)
Optional dropout to apply to the text field embeddings before passing through
the `seq2vec_encoder`.
"""

def __init__(
self,
vocab: Vocabulary,
text_field_embedder: TextFieldEmbedder,
seq2vec_encoder: Seq2SeqEncoder,
dropout: Optional[float] = None,
) -> None:
pass

def forward(
self,
tokens: TextFieldTensors,
labels: Optional[Tensor] = None,
) -> Dict[str, Tensor]:
"""
Runs a forward pass of the model, computing the predicted logits and also the loss
when `labels` is provided.
# Parameters
tokens : `TextFieldTensors`
The tokens corresponding to the source sequence.
labels : `Optional[Tensor]`, optional (default = `None`)
The target labels.
# Returns
`Dict[str, Tensor]`
An output dictionary with keys for the `loss` and `logits`.
"""
pass
```

### New models

**Do you have a new state-of-the-art model?**
Expand Down
1 change: 1 addition & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ coverage:
project:
default:
threshold: 1%
informational: true
changes: false
comment: false
ignore:
Expand Down

0 comments on commit b1b455a

Please sign in to comment.