Skip to content

Enable mypy in precommit checks #2177

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

Merged
merged 22 commits into from
Jun 25, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Add CI notes to CONTRIBUTING.md
  • Loading branch information
chriselion authored Jun 25, 2019
commit 3e90e0e7662db55beeea85eb7d94b75fe244b36a
16 changes: 14 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ the platform, and provide a unique non-trivial challenge to modern
machine learning algorithms. Feel free to submit these environments with a
PR explaining the nature of the environment and task.

## Style Guide
## Continuous Integration (CI)
Copy link
Contributor

@xiaomaogy xiaomaogy Jun 25, 2019

Choose a reason for hiding this comment

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

I think this is still a style guide, we just enforce our style through CI, so maybe still keep this as Style Guide?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a "Code Style" section below. I wanted to give a little more explanation about how the checks (black, mypy, and anything else we add later) were run. I can change this back and move it to another doc if you think it was better before.

Copy link
Contributor

Choose a reason for hiding this comment

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

No I think everything is fine here. I don't have a strong opinion on this.


When performing changes to the codebase, please ensure that all python code is reformatted using the [black](https://github.com/ambv/black) formatter. For C#, we will soon be requirements for style and formatting.
We run CircleCI on all PRs; all tests must be passing before the PR is merged.

Several static checks are run on the codebase using the [pre-commit framework](https://pre-commit.com/) during CI. To execute the same checks locally, install `pre-commit` and run `pre-commit run --all-files`. Some hooks (for example, `black`) will output the corrected version of the code; others (like `mypy`) may require more effort to fix.

### Code style
All python code should be formatted with [`black`](https://github.com/ambv/black). Style and formatting for C# may be enforced later.

### Python type annotations
We use [`mypy`](http://mypy-lang.org/) to perform static type checking on python code. Currently not all code is annotated but we will increase coverage over time. If you are adding or refactoring code, please
1. Add type annotations to the new or refactored code.
2. Make sure that code calling or called by the modified code also has type annotations.

The [type hint cheat sheet](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html) provides a good introduction to adding type hints.