From 8cd2d692beff576779f34601e7eff1fb5ccff76a Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Mon, 4 Oct 2021 18:49:58 +0100 Subject: [PATCH 1/3] Enhanced contributing guide --- CONTRIBUTING.md | 56 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7a120b234ee..2c1f175fd24 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -75,21 +75,57 @@ If you would like to contribute a new dataset, please see [here](#New-dataset). ### Code formatting and typing -Contributions should be compatible with Python 3.X versions and be compliant with PEP8. To check the codebase, please -either run +#### Formatting + +The torchvision code is formatted by [black](https://black.readthedocs.io/en/stable/), +and checked against pep8 compliance with [flake8](https://flake8.pycqa.org/en/latest/). + +To format your code, you can install [ufmt](https://github.com/omnilib/ufmt) +with `pip install ufmt` and format your code with e.g.: + ```bash -pre-commit run --all-files +ufmt format torchvision ``` -or run + +For the formatting to be a bit faster, you can also choose to only apply `ufmt` +to the files that were edited in your PR with e.g.: + ```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 +ufmt format `git diff main --name-only` ``` +You can also set your editor to run `ufmt format` every time you save a file. + +Note: the reason we rely on `ufmt` instead of just `black` is for the formatting +to be fully compatible with Facebook internal formatting tools, although using +`black` directly should work in most cases. Feel free to try and our linting CI +will tell you if something is wrong. + +Similarly, you can check for flake8 errors with `flake8 torchvision`, although +`flake8` errors should be fairly rare considering that most of them are +automatically taken care of by `ufmt` already. + +##### Pre-commit hooks + +For convenience and purely optionally, you can rely on our [pre-commit +hooks](https://pre-commit.com/) which will run both `ufmt` and `flake8` prior to +every commit. + +First install the `pre-commit` package with `pip install pre-commit`, and then +run `pre-commit install` at the root of the repo for the hooks to be set up - +that's it. + +Feel free to check out the [pre-commit docs](https://pre-commit.com/#usage) to +learn more and improve your workflow. You'll see for example that `pre-commit +run --all-files` will run both `ufmt` and `flake8` without the need for you to +commit anything, and that the `--no-verify` flag can be added when committing to +deactivate the hooks. + +Again, pre-commit hooks are entirely optional: feel free to manually call `ufmt` +and `flake8` manually instead. + +#### Type annotations + The codebase has type annotations, please make sure to add type hints if required. We use `mypy` tool for type checking: ```bash mypy --config-file mypy.ini From 648533aa81617f930d11b83e5f320aa9680dbae5 Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Mon, 4 Oct 2021 18:56:40 +0100 Subject: [PATCH 2/3] Some more --- CONTRIBUTING.md | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2c1f175fd24..b3669cc0c27 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,16 +79,19 @@ If you would like to contribute a new dataset, please see [here](#New-dataset). The torchvision code is formatted by [black](https://black.readthedocs.io/en/stable/), and checked against pep8 compliance with [flake8](https://flake8.pycqa.org/en/latest/). +Instead of relying directly on `black` however, we rely on +[ufmt](https://github.com/omnilib/ufmt), for compatibility reasons with Facebook +internal infra. -To format your code, you can install [ufmt](https://github.com/omnilib/ufmt) -with `pip install ufmt` and format your code with e.g.: +To format your code, install `ufmt` with `pip install ufmt` and use e.g.: ```bash ufmt format torchvision ``` -For the formatting to be a bit faster, you can also choose to only apply `ufmt` -to the files that were edited in your PR with e.g.: +For the vast majority of cases, this is all you should need to run. For the +formatting to be a bit faster, you can also choose to only apply `ufmt` to the +files that were edited in your PR with e.g.: ```bash ufmt format `git diff main --name-only` @@ -96,14 +99,9 @@ ufmt format `git diff main --name-only` You can also set your editor to run `ufmt format` every time you save a file. -Note: the reason we rely on `ufmt` instead of just `black` is for the formatting -to be fully compatible with Facebook internal formatting tools, although using -`black` directly should work in most cases. Feel free to try and our linting CI -will tell you if something is wrong. - -Similarly, you can check for flake8 errors with `flake8 torchvision`, although -`flake8` errors should be fairly rare considering that most of them are -automatically taken care of by `ufmt` already. +Similarly, you can check for `flake8` errors with `flake8 torchvision`, although +they should be fairly rare considering that most of the errors are automatically +taken care of by `ufmt` already. ##### Pre-commit hooks @@ -115,11 +113,11 @@ First install the `pre-commit` package with `pip install pre-commit`, and then run `pre-commit install` at the root of the repo for the hooks to be set up - that's it. -Feel free to check out the [pre-commit docs](https://pre-commit.com/#usage) to -learn more and improve your workflow. You'll see for example that `pre-commit -run --all-files` will run both `ufmt` and `flake8` without the need for you to -commit anything, and that the `--no-verify` flag can be added when committing to -deactivate the hooks. +Feel free to read the [pre-commit docs](https://pre-commit.com/#usage) to learn +more and improve your workflow. You'll see for example that `pre-commit run +--all-files` will run both `ufmt` and `flake8` without the need for you to +commit anything, and that the `--no-verify` flag can be added to `git commit` to +temporarily deactivate the hooks. Again, pre-commit hooks are entirely optional: feel free to manually call `ufmt` and `flake8` manually instead. From cb1b7f1ce8e2d3963c8c0ab6fa122c72be802d8f Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Tue, 5 Oct 2021 12:57:16 +0100 Subject: [PATCH 3/3] Address comments --- CONTRIBUTING.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b3669cc0c27..612827a99b5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ The torchvision code is formatted by [black](https://black.readthedocs.io/en/sta and checked against pep8 compliance with [flake8](https://flake8.pycqa.org/en/latest/). Instead of relying directly on `black` however, we rely on [ufmt](https://github.com/omnilib/ufmt), for compatibility reasons with Facebook -internal infra. +internal infrastructure. To format your code, install `ufmt` with `pip install ufmt` and use e.g.: @@ -97,15 +97,13 @@ files that were edited in your PR with e.g.: ufmt format `git diff main --name-only` ``` -You can also set your editor to run `ufmt format` every time you save a file. - Similarly, you can check for `flake8` errors with `flake8 torchvision`, although they should be fairly rare considering that most of the errors are automatically taken care of by `ufmt` already. ##### Pre-commit hooks -For convenience and purely optionally, you can rely on our [pre-commit +For convenience and **purely optionally**, you can rely on [pre-commit hooks](https://pre-commit.com/) which will run both `ufmt` and `flake8` prior to every commit. @@ -119,9 +117,6 @@ more and improve your workflow. You'll see for example that `pre-commit run commit anything, and that the `--no-verify` flag can be added to `git commit` to temporarily deactivate the hooks. -Again, pre-commit hooks are entirely optional: feel free to manually call `ufmt` -and `flake8` manually instead. - #### Type annotations The codebase has type annotations, please make sure to add type hints if required. We use `mypy` tool for type checking: