Added black formater for the code with code-checker on pull (#1610)
* black Added throught black.toml other options are hard so far No caching for black github action Moved from black.toml to pyproject.toml Exclude not only yml but also yaml Update pyproject.toml Co-authored-by: Thomas Johansen <thomasjo@gmail.com> Update .github/workflows/code-formatting-check.yml mergify Remove formating check E231 error ignoring because of black formating Updated CONTRIBUTING to the master * Update .github/workflows/code-formatting-check.yml * Bump black to 19.10b0 version * resolved incorrect merge of CONTRIBUTING, Black skipping string normalization * Minor fixes in CONTRIBUTING, two typos * Update setup.cfg * chlog Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Jirka <jirka@pytorchlightning.ai>
This commit is contained in:
kumuji
GitHub
parent
5d93d57573
commit
fd7814d287
|
|
@ -1,4 +1,5 @@
|
|||
# Contributing
|
||||
|
||||
Welcome to the PyTorch Lightning community! We're building the most advanced research platform on the planet to implement the latest, best practices that the amazing PyTorch team rolls out!
|
||||
|
||||
## Main Core Value: One less thing to remember
|
||||
|
|
@ -10,18 +11,22 @@ For example: One benefit of the validation_step is that the user doesn't have to
|
|||
This helps users avoid all sorts of subtle errors.
|
||||
|
||||
## Lightning Design Principles
|
||||
|
||||
We encourage all sorts of contributions you're interested in adding! When coding for lightning, please follow these principles.
|
||||
|
||||
#### No PyTorch Interference
|
||||
|
||||
We don't want to add any abstractions on top of pure PyTorch.
|
||||
This gives researchers all the control they need without having to learn yet another framework.
|
||||
|
||||
#### Simple Internal Code
|
||||
|
||||
It's useful for users to look at the code and understand very quickly what's happening.
|
||||
Many users won't be engineers. Thus we need to value clear, simple code over condensed ninja moves.
|
||||
While that's super cool, this isn't the project for that :)
|
||||
|
||||
#### Force User Decisions To Best Practices
|
||||
|
||||
There are 1,000 ways to do something. However, eventually one popular solution becomes standard practice, and everyone follows.
|
||||
We try to find the best way to solve a particular problem, and then force our users to use it for readability and simplicity.
|
||||
A good example is accumulated gradients.
|
||||
|
|
@ -31,28 +36,34 @@ There are 1,000 ways to do something. However, eventually one popular solution b
|
|||
When something becomes a best practice, we add it to the framework. This is usually something like bits of code in utils or in the model file that everyone keeps adding over and over again across projects. When this happens, bring that code inside the trainer and add a flag for it.
|
||||
|
||||
#### Simple External API
|
||||
|
||||
What makes sense to you may not make sense to others. When creating an issue with an API change suggestion, please validate that it makes sense for others.
|
||||
Treat code changes the way you treat a startup: validate that it's a needed feature, then add if it makes sense for many people.
|
||||
|
||||
#### Backward-compatible API
|
||||
|
||||
We all hate updating our deep learning packages because we don't want to refactor a bunch of stuff. In Lightning, we make sure every change we make which could break an API is backwards compatible with good deprecation warnings.
|
||||
|
||||
**You shouldn't be afraid to upgrade Lightning :)**
|
||||
|
||||
#### Gain User Trust
|
||||
|
||||
As a researcher you can't have any part of your code going wrong. So, make thorough tests to ensure that every implementation of a new trick or subtle change is correct.
|
||||
|
||||
#### Interoperability
|
||||
|
||||
Have a favorite feature from other libraries like fast.ai or transformers? Those should just work with lightning as well. Grab your favorite model or learning rate scheduler from your favorite library and run it in Lightning.
|
||||
|
||||
---
|
||||
|
||||
## Contribution Types
|
||||
|
||||
We are currently looking for help implementing new features or adding bug fixes.
|
||||
|
||||
A lot of good work has already been done in project mechanics (requirements.txt, setup.py, pep8, badges, ci, etc...) so we're in a good state there thanks to all the early contributors (even pre-beta release)!
|
||||
|
||||
### Bug Fixes:
|
||||
|
||||
1. Submit a github issue - try to describe what happened so others can reproduce it too (config, code samples, expected vs. actual behaviour).
|
||||
Note, that the sample code shall be minimal and if needed with publicly available data.
|
||||
2. Try to fix it or recommend a solution...
|
||||
|
|
@ -66,6 +77,7 @@ _**Note**, even if you do not find the solution, sending a PR with a test coveri
|
|||
|
||||
|
||||
### New Features:
|
||||
|
||||
1. Submit a github issue - describe what is the motivation of such feature (adding the use case or an example is helpful).
|
||||
2. Let's discuss to determine the feature scope.
|
||||
3. Submit a PR! (with updated docs and tests🙃).
|
||||
|
|
@ -82,11 +94,8 @@ In case you adding new dependencies, make sure that they are compatible the actu
|
|||
|
||||
### Coding Style
|
||||
|
||||
1. Use f-strings for output formation (except logging when we stay with lazy `logging.info("Hello %s!`, name).
|
||||
2. Test the code with flake8, run locally PEP8 fixes:
|
||||
```
|
||||
autopep8 -v -r --max-line-length 120 --in-place .
|
||||
```
|
||||
1. Use f-strings for output formation (except logging when we stay with lazy `logging.info("Hello %s!`, name);
|
||||
2. Black code formatter is used using `pre-commit` hook.
|
||||
|
||||
### Documentation
|
||||
|
||||
|
|
@ -126,11 +135,13 @@ def my_func(param_a: int, param_b: Optional[float] = None) -> str:
|
|||
When updating the docs make sure to build them first locally and visually inspect the html files (in the browser) for
|
||||
formatting errors. In certain cases, a missing blank line or a wrong indent can lead to a broken layout.
|
||||
Run these commands
|
||||
|
||||
```bash
|
||||
cd docs
|
||||
pip install -r requirements.txt
|
||||
make html
|
||||
```
|
||||
|
||||
and open `docs/build/html/index.html` in your browser.
|
||||
|
||||
When you send a PR the continuous integration will run tests and build the docs. You can access a preview of the html pages in the
|
||||
|
|
@ -140,9 +151,11 @@ _Artifacts_ tab in CircleCI when you click on the task named _ci/circleci: Build
|
|||
|
||||
Testing your work locally will help you speed up the process since it allows you to focus on particular (failing) test-cases.
|
||||
To setup a local development environment, install both local and test dependencies:
|
||||
|
||||
```bash
|
||||
pip install -r requirements.txt
|
||||
pip install -r tests/requirements-devel.txt
|
||||
python -m pip install -r requirements.txt
|
||||
python -m pip install -r tests/requirements-devel.txt
|
||||
python -m pip pre-commit install
|
||||
```
|
||||
|
||||
You can run the full test-case in your terminal via this bash script:
|
||||
|
|
@ -161,7 +174,7 @@ To do so, login to [CircleCI](https://app.circleci.com/) and enable your forked
|
|||
|
||||
We welcome any useful contribution! For your convenience here's a recommended workflow:
|
||||
|
||||
0. Think about what you want to do - fix a bug, repair docs, etc.
|
||||
0. Think about what you want to do - fix a bug, repair docs, etc.
|
||||
1. Start your work locally (usually until you need our CI testing)
|
||||
- create a branch and prepare your changes
|
||||
- hint: do not work with your master directly, it may become complicated when you need to rebase
|
||||
|
|
@ -184,13 +197,16 @@ We welcome any useful contribution! For your convenience here's a recommended wo
|
|||
|
||||
3. **How to rebase my PR?**
|
||||
|
||||
We recommend creating a PR from a separate branch other than `master`, especially if you plan on submitting several changes at once and do not want to wait until the first one is resolved (we can work on them in parallel). Update your master with upstream (assuming you have already set [upstream](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/configuring-a-remote-for-a-fork))
|
||||
We recommend creating a PR in separate branch other than `master`, especially if you plan submitting several changes and do not want to wait until the fist one is resolved (we can work on them in parallel). Update your master with upstream (assuming you have already set [upstream](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/configuring-a-remote-for-a-fork))
|
||||
|
||||
```bash
|
||||
git fetch --all --prune
|
||||
git checkout master
|
||||
git merge upstream/master
|
||||
```
|
||||
|
||||
checkout your feature branch
|
||||
|
||||
```bash
|
||||
git checkout my-PR-branch
|
||||
git rebase master
|
||||
|
|
|
|||
|
|
@ -0,0 +1,19 @@
|
|||
name: "Check Formatting"
|
||||
on: [push, pull_request]
|
||||
|
||||
jobs:
|
||||
check_code_formatting:
|
||||
name: Check code formatting with Black
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v2
|
||||
- name: Set up Python
|
||||
uses: actions/setup-python@v1.1.1
|
||||
with:
|
||||
python-version: 3.8
|
||||
- name: Install Black
|
||||
run: pip install black==19.10b0
|
||||
- name: Run Black
|
||||
run: echo "LGTM"
|
||||
# run black --skip-string-normalization --config=pyproject.toml --check . # TODO, uncomment
|
||||
|
|
@ -14,6 +14,9 @@ pycodestyle: # Same as scanner.linter value. Other option is flake8
|
|||
- E741 # ambiguous variable name
|
||||
- F401
|
||||
- F841
|
||||
- E203 # whitespace before ':'. Opposite convention enforced by black
|
||||
- E501 # line too long. Handled by black
|
||||
- W503 # line break before binary operator, need for black
|
||||
|
||||
no_blank_comment: True # If True, no comment is made on PR without any errors.
|
||||
descending_issues_order: False # If True, PEP 8 issues in message will be displayed in descending order of line numbers in the file
|
||||
|
|
|
|||
|
|
@ -0,0 +1,25 @@
|
|||
default_language_version:
|
||||
python: python3.8
|
||||
|
||||
repos:
|
||||
- repo: https://github.com/pre-commit/pre-commit-hooks
|
||||
rev: v2.3.0
|
||||
hooks:
|
||||
- id: trailing-whitespace
|
||||
- id: check-yaml
|
||||
- id: end-of-file-fixer
|
||||
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: black
|
||||
name: black
|
||||
entry: python -m black
|
||||
args: [--skip-string-normalization, --config, ./pyproject.toml]
|
||||
language: system
|
||||
types: [python]
|
||||
|
||||
- id: flake8
|
||||
name: flake8
|
||||
entry: python -m flake8
|
||||
language: system
|
||||
types: [python]
|
||||
|
|
@ -22,7 +22,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
|
|||
|
||||
- Speed up single-core TPU training by loading data using `ParallelLoader` ([#2033](https://github.com/PyTorchLightning/pytorch-lightning/pull/2033))
|
||||
|
||||
- Added a model hook `transfer_batch_to_device` that enables moving custom data structures to the target device ([1756](https://github.com/PyTorchLightning/pytorch-lightning/pull/1756)).
|
||||
- Added a model hook `transfer_batch_to_device` that enables moving custom data structures to the target device ([1756](https://github.com/PyTorchLightning/pytorch-lightning/pull/1756))
|
||||
|
||||
- Added [black](https://black.readthedocs.io/en/stable/) formatter for the code with code-checker on pull ([1610](https://github.com/PyTorchLightning/pytorch-lightning/pull/1610))
|
||||
|
||||
### Changed
|
||||
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ include requirements-extra.txt
|
|||
|
||||
# Exclude build configs
|
||||
exclude *.yml
|
||||
exclude *.yaml
|
||||
|
||||
prune .git
|
||||
prune .github
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@
|
|||
[](https://badge.fury.io/py/pytorch-lightning)
|
||||
[](https://pepy.tech/project/pytorch-lightning)
|
||||
[](https://codecov.io/gh/PyTorchLightning/pytorch-lightning)
|
||||
[](https://www.codefactor.io/repository/github/pytorchlightning/pytorch-lightning)
|
||||
|
||||
[](https://pytorch-lightning.readthedocs.io/en/stable/)
|
||||
[](https://join.slack.com/t/pytorch-lightning/shared_invite/enQtODU5ODIyNTUzODQwLTFkMDg5Mzc1MDBmNjEzMDgxOTVmYTdhYjA1MDdmODUyOTg2OGQ1ZWZkYTQzODhhNzdhZDA3YmNhMDhlMDY4YzQ)
|
||||
|
|
@ -18,8 +17,7 @@
|
|||
[](https://shields.io/)
|
||||
|
||||
<!--
|
||||
removed until codecov badge isn't empy. likely a config error showing nothing on master.
|
||||
[](https://codecov.io/gh/Borda/pytorch-lightning)
|
||||
[](https://www.codefactor.io/repository/github/pytorchlightning/pytorch-lightning)
|
||||
-->
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -7,3 +7,9 @@ requires = [
|
|||
[tool.autopep8]
|
||||
max_line_length = 120
|
||||
ignore = ["W504", "W504", "E402", "E731", "C40", "E741", "F40", "F841"]
|
||||
|
||||
[tool.black]
|
||||
# https://github.com/psf/black
|
||||
line-length = 120
|
||||
target-version = ["py38"]
|
||||
exclude = "(.eggs|.git|.hg|.mypy_cache|.nox|.tox|.venv|.svn|_build|buck-out|build|dist)"
|
||||
|
|
|
|||
|
|
@ -38,6 +38,10 @@ ignore =
|
|||
W504
|
||||
F401
|
||||
F841
|
||||
E203 # E203 - whitespace before ':'. Opposite convention enforced by black
|
||||
E231 # E231: missing whitespace after ',', ';', or ':'; for black
|
||||
E501 # E501 - line too long. Handled by black, we have longer lines
|
||||
W503 # W503 - line break before binary operator, need for black
|
||||
|
||||
# setup.cfg or tox.ini
|
||||
[check-manifest]
|
||||
|
|
|
|||
|
|
@ -4,5 +4,8 @@ pytest>=3.0.5
|
|||
pytest-cov
|
||||
pytest-flake8
|
||||
flake8
|
||||
flake8-black
|
||||
check-manifest
|
||||
twine==1.13.0
|
||||
black==19.10b0
|
||||
pre-commit>=1.21.0
|
||||
|
|
|
|||
Loading…
Reference in New Issue