Skip to content
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

add pre-commit hooks for convenient formatting checks #4387

Merged
merged 8 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 3 additions & 2 deletions .circleci/config.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions .circleci/config.yml.in
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,9 @@ jobs:
- checkout
- run:
command: |
pip install --user --progress-bar off flake8 typing
flake8 --config=setup.cfg .
pip install --user --progress-bar off pre-commit
pre-commit install-hooks
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can delete this line?
I think run --all-files should work even without having the hooks? At least that's what our instructions in the README suggest.

CI failures are unrelated BTW, it's good to merge on my side. I'll let you do it in case there's any leftovers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like to keep the actual check separate from the installation. This way, the last step in the workflow only contains the information that a contributor needs in case the workflow fails. Granted, the installation of the hook environments does not create a lot of noise, but still.

Copy link
Contributor

Choose a reason for hiding this comment

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

- run: pre-commit run --all-files

python_type_check:
docker:
Expand Down
4 changes: 2 additions & 2 deletions .circleci/unittest/linux/scripts/run-clang-format.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
"""
"""A wrapper script around clang-format, suitable for linting multiple files
A wrapper script around clang-format, suitable for linting multiple files
and to use for continuous integration.
This is an alternative API for the clang-format command line.
Expand Down
2 changes: 1 addition & 1 deletion .circleci/unittest/windows/scripts/install_conda.bat
Original file line number Diff line number Diff line change
@@ -1 +1 @@
start /wait "" "%miniconda_exe%" /S /InstallationType=JustMe /RegisterPython=0 /AddToPath=0 /D=%tmp_conda%
start /wait "" "%miniconda_exe%" /S /InstallationType=JustMe /RegisterPython=0 /AddToPath=0 /D=%tmp_conda%
14 changes: 14 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
repos:
- repo: https://gitlab.com/pycqa/flake8
pmeier marked this conversation as resolved.
Show resolved Hide resolved
rev: 3.9.2
hooks:
- id: flake8
args: [--config=setup.cfg]
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
hooks:
- id: check-docstring-first
- id: check-toml
- id: check-yaml
exclude: packaging/.*
- id: end-of-file-fixer
16 changes: 13 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ If you plan to modify the code or documentation, please follow the steps below:
2. If you have modified the code (new feature or bug-fix), please add unit tests.
3. If you have changed APIs, update the documentation. Make sure the documentation builds.
4. Ensure the test suite passes.
5. Make sure your code passes `flake8` formatting check.
5. Make sure your code passes the formatting checks (see below).

For more details about pull requests,
please read [GitHub's guides](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request).
Expand All @@ -75,9 +75,19 @@ If you would like to contribute a new dataset, please see [here](#New-dataset).

### Code formatting and typing

New code should be compatible with Python 3.X versions and be compliant with PEP8. To check the codebase, please run
Contributions should be compatible with Python 3.X versions and be compliant with PEP8. To check the codebase, please
either run
```bash
flake8 --config=setup.cfg .
pre-commit run --all-files
```
or run
```bash
pre-commit install
```
once to perform these checks automatically before every `git commit`. If `pre-commit` is not available you can install
it with
```
pip install pre-commit
```

The codebase has type annotations, please make sure to add type hints if required. We use `mypy` tool for type checking:
Expand Down
1 change: 0 additions & 1 deletion android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,3 @@ allprojects {
ext.deps = [
jsr305: 'com.google.code.findbugs:jsr305:3.0.1',
]

2 changes: 1 addition & 1 deletion android/test_app/app/src/main/res/layout/activity_main.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@
android:background="@android:color/black"
android:textColor="@android:color/white" />

</FrameLayout>
</FrameLayout>
2 changes: 1 addition & 1 deletion docs/source/_static/css/custom_torchvision.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ torchvision it just hides the links. So we have to put them back here */
article.pytorch-article .sphx-glr-download-link-note.admonition.note,
article.pytorch-article .reference.download.internal, article.pytorch-article .sphx-glr-signature {
display: block;
}
}
2 changes: 1 addition & 1 deletion docs/source/_static/img/pytorch-logo-flame.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion docs/source/feature_extraction.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,4 @@ API Reference

.. autofunction:: create_feature_extractor

.. autofunction:: get_graph_node_names
.. autofunction:: get_graph_node_names
2 changes: 1 addition & 1 deletion examples/cpp/hello_world/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ Once both dependencies are sorted, we can start the CMake fun:

| That's it!
| You should now have a ``hello-world`` executable in your ``build`` folder.
Running it will output a (fairly long) tensor of random values to your terminal.
Running it will output a (fairly long) tensor of random values to your terminal.
2 changes: 1 addition & 1 deletion examples/python/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Python examples

The examples in this directory have been moved online in our [gallery
page](https://pytorch.org/vision/stable/auto_examples/index.html).
page](https://pytorch.org/vision/stable/auto_examples/index.html).
2 changes: 1 addition & 1 deletion gallery/README.rst
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Example gallery
===============

Below is a gallery of examples
Below is a gallery of examples
2 changes: 1 addition & 1 deletion gallery/assets/imagenet_class_index.json

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion ios/VisionTestApp/VisionTestApp/ViewController.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@


@end

1 change: 0 additions & 1 deletion packaging/vs2017/install_activate.bat
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,3 @@ IF "%cross_compiler_target_platform%" == "win-64" (
echo CALL "VC\Auxiliary\Build\vcvars32.bat" >> "%PREFIX%\etc\conda\activate.d\vs%YEAR%_compiler_vars.bat"
echo popd
)

1 change: 0 additions & 1 deletion packaging/vs2019/install_activate.bat
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,3 @@ IF "%cross_compiler_target_platform%" == "win-64" (
echo CALL "VC\Auxiliary\Build\vcvars32.bat" >> "%PREFIX%\etc\conda\activate.d\vs%YEAR%_compiler_vars.bat"
echo popd
)

1 change: 0 additions & 1 deletion references/classification/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,3 @@ For post training quant, device is set to CPU. For training, the device is set t
```
python train_quantization.py --device='cpu' --test-only --backend='<backend>' --model='<model_name>'
```

1 change: 0 additions & 1 deletion references/detection/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,3 @@ python -m torch.distributed.launch --nproc_per_node=8 --use_env train.py\
--dataset coco_kp --model keypointrcnn_resnet50_fpn --epochs 46\
--lr-steps 36 43 --aspect-ratio-group-factor 3
```

4 changes: 2 additions & 2 deletions test/test_video_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@
"DecoderResult", "vframes vframe_pts vtimebase aframes aframe_pts atimebase"
)

"""av_seek_frame is imprecise so seek to a timestamp earlier by a margin
The unit of margin is second"""
# av_seek_frame is imprecise so seek to a timestamp earlier by a margin
# The unit of margin is second
seek_frame_margin = 0.25


Expand Down