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

Use ruff #1994

Merged
merged 19 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 17 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
11 changes: 2 additions & 9 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,16 @@
"customizations": {
"vscode": {
"extensions": [
"GitHub.codespaces",
"charliermarsh.ruff",
"ms-python.python",
"ms-python.isort",
"ms-python.black-formatter",
"ms-python.vscode-pylance",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got rid of pylance since we aren't currently using it anywhere else. Also, haven't tested this file since I don't know how to use it.

"ms-python.pylint",
"ms-python.flake8",
"ms-python.mypy-type-checker",
"GitHub.codespaces",
"ms-toolsai.jupyter"
],
"settings": {
"telemetry.telemetryLevel": "off",
"python.languageServer": "Pylance",
"python.defaultInterpreterPath": "/usr/local/bin/python",
"python.formatting.blackPath": "/usr/local/py-utils/bin/black",
"python.testing.pytestEnabled": true,
"python.testing.pytestPath": "/usr/local/py-utils/bin/pytest",
"python.editor.formatOnSave": true,
Expand All @@ -44,8 +39,6 @@
"source.organizeImports": true
},
"python.diffEditor.wordWrap": "off",
"isort.args": ["--profile", "black"],
"isort.path": ["/home/vscode/.local/bin/isort"]
}
}
}
Expand Down
31 changes: 0 additions & 31 deletions .flake8

This file was deleted.

112 changes: 7 additions & 105 deletions .github/workflows/style.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,81 +9,6 @@ on:
- main
- release**
jobs:
black:
name: black
runs-on: ubuntu-latest
steps:
- name: Clone repo
uses: actions/[email protected]
- name: Set up python
uses: actions/[email protected]
with:
python-version: '3.12'
- name: Cache dependencies
uses: actions/[email protected]
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/style.txt') }}
- name: Install pip dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: |
pip install -r requirements/style.txt
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run black checks
run: black . --check --diff
flake8:
name: flake8
runs-on: ubuntu-latest
steps:
- name: Clone repo
uses: actions/[email protected]
- name: Set up python
uses: actions/[email protected]
with:
python-version: '3.12'
- name: Cache dependencies
uses: actions/[email protected]
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/style.txt') }}
- name: Install pip dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: |
pip install -r requirements/style.txt
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run flake8 checks
run: flake8
isort:
name: isort
runs-on: ubuntu-latest
steps:
- name: Clone repo
uses: actions/[email protected]
- name: Set up python
uses: actions/[email protected]
with:
python-version: '3.12'
- name: Cache dependencies
uses: actions/[email protected]
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/style.txt') }}
- name: Install pip dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: |
pip install -r requirements/style.txt
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run isort checks
run: isort . --check --diff
mypy:
name: mypy
runs-on: ubuntu-latest
Expand All @@ -99,18 +24,18 @@ jobs:
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/required.txt') }}-${{ hashFiles('requirements/datasets.txt') }}-${{ hashFiles('requirements/tests.txt') }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/required.txt') }}-${{ hashFiles('requirements/datasets.txt') }}-${{ hashFiles('requirements/style.txt') }}
- name: Install pip dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: |
pip install -r requirements/required.txt -r requirements/datasets.txt -r requirements/tests.txt
pip install -r requirements/required.txt -r requirements/datasets.txt -r requirements/style.txt
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run mypy checks
run: mypy .
pydocstyle:
name: pydocstyle
ruff:
name: ruff
runs-on: ubuntu-latest
steps:
- name: Clone repo
Expand All @@ -132,33 +57,10 @@ jobs:
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run pydocstyle checks
run: pydocstyle
pyupgrade:
name: pyupgrade
runs-on: ubuntu-latest
steps:
- name: Clone repo
uses: actions/[email protected]
- name: Set up python
uses: actions/[email protected]
with:
python-version: '3.12'
- name: Cache dependencies
uses: actions/[email protected]
id: cache
with:
path: ${{ env.pythonLocation }}
key: ${{ env.pythonLocation }}-${{ hashFiles('requirements/style.txt') }}
- name: Install pip dependencies
if: steps.cache.outputs.cache-hit != 'true'
- name: Run ruff checks
run: |
pip install -r requirements/style.txt
pip cache purge
- name: List pip dependencies
run: pip list
- name: Run pyupgrade checks
run: pyupgrade --py310-plus $(find . -path ./docs/src -prune -o -name "*.py" -print)
ruff check --output-format=github --no-fix .
isaaccorley marked this conversation as resolved.
Show resolved Hide resolved
ruff format --diff
concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.head.label || github.head_ref || github.ref }}
cancel-in-progress: true
71 changes: 34 additions & 37 deletions .pre-commit-config.yaml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ashnair1 can you test this?

I'm wondering if we should remove mypy from pre-commit since it's so slow.

We could also add the pre-commit action so that fixes are automatically applied to PRs.

Copy link
Collaborator

@ashnair1 ashnair1 Apr 14, 2024

Choose a reason for hiding this comment

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

Tested the file and it works fine.

The slowness observed in the mypy hook occurs when there is a change in the additional dependencies as the isolated environment (where pre-commit runs the hooks) needs to be recreated. Once it's done, subsequent runs will be faster.

Incorporating the pre-commit action is a good idea. That way, new contributors won't have to worry about style. From a CI perspective, since we already have a Github action for checking mypy, we theoretically wouldn't need a mypy hook in the pre-commit config anymore.

On the flip side, having the mypy hook in the pre-commit config is useful for testing mypy when you don't have the latest versions of the libraries installed locally :)

Original file line number Diff line number Diff line change
@@ -1,38 +1,35 @@
repos:
- repo: https://github.com/asottile/pyupgrade
rev: v3.15.2
hooks:
- id: pyupgrade
args: [--py310-plus]

- repo: https://github.com/pycqa/isort
rev: 5.13.2
hooks:
- id: isort
additional_dependencies: ['.[colors]']

- repo: https://github.com/psf/black
rev: 24.3.0
hooks:
- id: black
args: [--skip-magic-trailing-comma]

- repo: https://github.com/pycqa/flake8.git
rev: 7.0.0
hooks:
- id: flake8

- repo: https://github.com/pycqa/pydocstyle
rev: 6.3.0
hooks:
- id: pydocstyle
exclude: (tests|docs|experiments)
additional_dependencies: ['.[toml]']

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.9.0
hooks:
- id: mypy
args: [--strict, --ignore-missing-imports, --show-error-codes]
additional_dependencies: [einops>=0.6.0, kornia>=0.6.9, lightning>=2.0.9, matplotlib>=3.8.1, numpy>=1.22, pillow>=10.3.0, pytest>=6.1.2, pyvista>=0.34.2, scikit-image>=0.18.0, torch>=2.2, torchmetrics>=0.10]
exclude: (build|data|dist|logo|logs|output)/
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.3.7
hooks:
- id: ruff
types_or:
- python
- jupyter
args:
- --fix
- id: ruff-format
types_or:
- python
- jupyter
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.9.0
hooks:
- id: mypy
args:
- --strict
- --ignore-missing-imports
- --show-error-codes
additional_dependencies:
- einops>=0.6.0
- kornia>=0.6.9
- lightning>=2.0.9
- matplotlib>=3.8.1
- numpy>=1.22
- pillow>=10.3.0
- pytest>=6.1.2
- pyvista>=0.34.2
- scikit-image>=0.18.0
- torch>=2.2
- torchmetrics>=0.10
exclude: (build|data|dist|logo|logs|output)/
1 change: 1 addition & 0 deletions docs/tutorials/custom_raster_dataset.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
"import pystac\n",
"import torch\n",
"from torch.utils.data import DataLoader\n",
"\n",
"from torchgeo.datasets import RasterDataset, stack_samples, unbind_samples\n",
"from torchgeo.datasets.utils import download_url\n",
"from torchgeo.samplers import RandomGeoSampler\n",
Expand Down
2 changes: 1 addition & 1 deletion docs/tutorials/indices.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"import matplotlib.pyplot as plt\n",
"\n",
"from torchgeo.datasets import EuroSAT100\n",
"from torchgeo.transforms import AppendNDVI, AppendNDWI, AppendNDBI"
"from torchgeo.transforms import AppendNDBI, AppendNDVI, AppendNDWI"
]
},
{
Expand Down
4 changes: 2 additions & 2 deletions docs/tutorials/pretrained_weights.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@
"from lightning.pytorch import Trainer\n",
"\n",
"from torchgeo.datamodules import EuroSAT100DataModule\n",
"from torchgeo.trainers import ClassificationTask\n",
"from torchgeo.models import ResNet18_Weights"
"from torchgeo.models import ResNet18_Weights\n",
"from torchgeo.trainers import ClassificationTask"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems like isort and pyupgrade did not have Jupyter notebook support

]
},
{
Expand Down
7 changes: 3 additions & 4 deletions docs/tutorials/transforms.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
"source": [
"import os\n",
"import tempfile\n",
"from typing import Dict, Optional\n",
"\n",
"import kornia.augmentation as K\n",
"import torch\n",
Expand Down Expand Up @@ -133,9 +132,9 @@
" def apply_transform(\n",
" self,\n",
" input: Tensor,\n",
" params: Dict[str, Tensor],\n",
" flags: Dict[str, int],\n",
" transform: Optional[Tensor] = None,\n",
" params: dict[str, Tensor],\n",
" flags: dict[str, int],\n",
" transform: Tensor | None = None,\n",
" ) -> Tensor:\n",
" return (input - flags[\"mins\"]) / (flags[\"maxs\"] - flags[\"mins\"] + 1e-10)"
]
Expand Down
21 changes: 7 additions & 14 deletions docs/user/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -88,36 +88,29 @@ These tests require `pytest <https://docs.pytest.org/>`_ and `pytest-cov <https:
Linters
-------

In order to remain `PEP-8 <https://peps.python.org/pep-0008/>`_ compliant and maintain a high-quality codebase, we use several linting tools:
In order to remain `PEP-8 <https://peps.python.org/pep-0008/>`_ compliant and maintain a high-quality codebase, we use a couple of linting tools:

* `black <https://black.readthedocs.io/>`_ for code formatting
* `isort <https://pycqa.github.io/isort/>`_ for import ordering
* `flake8 <https://flake8.pycqa.org/>`_ for code formatting
* `pydocstyle <https://www.pydocstyle.org/>`_ for docstrings
* `pyupgrade <https://github.com/asottile/pyupgrade>`_ for code formatting
* `ruff <https://docs.astral.sh/ruff/>`_ for code formatting
* `mypy <https://mypy.readthedocs.io/>`_ for static type analysis

All of these tools should be used from the root of the project to ensure that our configuration files are found. Black, isort, and pyupgrade are relatively easy to use, and will automatically format your code for you:
These tools should be used from the root of the project to ensure that our configuration files are found. Ruff is relatively easy to use, and will automatically fix most issues it encounters:

.. code-block:: console

$ black .
$ isort .
$ pyupgrade --py310-plus $(find . -name "*.py")
$ ruff check
$ ruff format


Flake8, pydocstyle, and mypy won't format your code for you, but they will warn you about potential issues with your code or docstrings:
Mypy won't fix your code for you, but will warn you about potential issues with your code:

.. code-block:: console

$ flake8
$ pydocstyle
$ mypy .


If you've never used mypy before or aren't familiar with `Python type hints <https://docs.python.org/3/library/typing.html>`_, this check can be particularly daunting. Don't hesitate to ask for help with resolving any of these warnings on your pull request.

You can also use `git pre-commit hooks <https://pre-commit.com/>`_ to automatically run these checks before each commit. pre-commit is a tool that automatically runs linters locally, so that you don't have to remember to run them manually and then have your code flagged by CI. You can setup pre-commit with:
You can also use `git pre-commit hooks <https://pre-commit.com/>`_ to automatically run these checks before each commit. pre-commit is a tool that automatically runs linters locally, so that you don't have to remember to run them manually and then have your code flagged by CI. You can set up pre-commit with:

.. code-block:: console

Expand Down
4 changes: 1 addition & 3 deletions experiments/ssl4eo/landsat/plot_landsat_bands.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@
df = df.iloc[::-1]

fig, ax = plt.subplots(figsize=(5.5, args.fig_height))
ax1, ax2 = fig.subplots(
nrows=1, ncols=2, gridspec_kw={"width_ratios": [3, 1]}
) # type: ignore[misc]
ax1, ax2 = fig.subplots(nrows=1, ncols=2, gridspec_kw={"width_ratios": [3, 1]}) # type: ignore[misc]
isaaccorley marked this conversation as resolved.
Show resolved Hide resolved

sensor_names: list[str] = []
sensor_ylocs: list[float] = []
Expand Down
Loading
Loading