Skip to content

Commit

Permalink
use lintrunner for formatters + pyre (#861)
Browse files Browse the repository at this point in the history
  • Loading branch information
d4l3k authored Mar 27, 2024
1 parent dd4c61f commit 2dbfec1
Show file tree
Hide file tree
Showing 13 changed files with 347 additions and 98 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,19 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.8
python-version: "3.10"
architecture: x64
- name: Checkout TorchX
uses: actions/checkout@v2
- name: Install Dependencies
run: |
set -eux
grep -E "(usort|black|flake8)" < dev-requirements.txt > /tmp/lint-requirements.txt
grep -E "(lintrunner)" < dev-requirements.txt > /tmp/lint-requirements.txt
pip install -r /tmp/lint-requirements.txt
lintrunner init
- name: Run Lint
run: |
git config --global url."https://${{ secrets.GITHUB_TOKEN }}:[email protected]/".insteadOf "https://github.com/"
scripts/lint.sh
lintrunner --skip PYRE --force-color --all-files
4 changes: 2 additions & 2 deletions .github/workflows/pyre.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
run: |
set -eux
pip install -e .[dev]
VERSION=$(grep "version" .pyre_configuration | sed -n -e 's/.*\(0\.0\.[0-9]*\).*/\1/p')
pip install pyre-check-nightly==$VERSION
lintrunner init
- name: Run Pyre
run: scripts/pyre.sh
43 changes: 43 additions & 0 deletions .lintrunner.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
[[linter]]
code = 'UFMT'
include_patterns = [
'**/*.py',
'**/*.pyi',
]
command = [
'python3',
'tools/linter/adapters/ufmt_linter.py',
'--',
'@{{PATHSFILE}}'
]
init_command = [
'python3',
'-m',
'lintrunner_adapters',
'run',
'pip_init',
'--dry-run={{DRYRUN}}',
'black==24.2.0',
'ufmt==2.5.1',
'usort==1.0.5',
]
is_formatter = true

[[linter]]
code = 'PYRE'
include_patterns = [
'**/*.py',
'**/*.pyi',
]
command = [
'python3',
'tools/linter/adapters/pyre_linter.py',
'--',
'@{{PATHSFILE}}'
]
init_command = [
'bash',
'scripts/setup_pyre.sh',
'--dry-run={{DRYRUN}}',
]
is_formatter = false
10 changes: 10 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ Facebook has a [bounty program](https://www.facebook.com/whitehat/) for the safe
disclosure of security bugs. In those cases, please go through the process
outlined on that page and do not file a public issue.

## Lint + Pyre

Lint and type checking can be run via `lintrunner`

```sh
pip install lintrunner lintrunner-adapters
lintrunner init
lintrunner -a
```

## Integration Tests

See the [KFP integration test](scripts/kfpint.py) file for more details on setup
Expand Down
6 changes: 4 additions & 2 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
aiobotocore==2.12.1
ax-platform[mysql]==0.2.3
black==22.12.0
boto3==1.34.51
captum>=0.4.0
docker
Expand Down Expand Up @@ -29,9 +28,12 @@ torchserve>=0.10.0
torchtext==0.17.1
torchvision==0.17.1
ts==0.5.1
usort==1.0.5
ray[default]

# lint (linter versions are managed by lintrunner)
lintrunner
lintrunner-adapters


# reduce backtracking
grpcio==1.62.1
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
[tool.usort]
first_party_detection = false

[tool.black]
target-version = ["py38", "py39", "py310", "py311"]
88 changes: 3 additions & 85 deletions scripts/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,92 +7,10 @@

set -e

if [ ! "$(black --version)" ]
if [ ! "$(lintrunner --version)" ]
then
echo "Please install black."
echo "Please install lintrunner."
exit 1
fi
if [ ! "$(usort --version)" ]
then
echo "Please install usort."
exit 1
fi
if [ ! "$(flake8 --version)" ]
then
echo "Please install flake8."
exit 1
fi

# cd to the project directory
cd "$(dirname "$0")/.." || exit 1

GIT_URL_1="https://github.com/pytorch/torchx.git"
GIT_URL_2="[email protected]:pytorch/torchx.git"

UPSTREAM_URL="$(git config remote.upstream.url)" || true

if [ -z "$UPSTREAM_URL" ]
then
echo "Setting upstream remote to $GIT_URL_1"
git remote add upstream "$GIT_URL_1"
elif [ "$UPSTREAM_URL" != "$GIT_URL_1" ] && \
[ "$UPSTREAM_URL" != "$GIT_URL_2" ]
then
echo "upstream remote set to $UPSTREAM_URL."
echo "Please delete the upstream remote or set it to $GIT_URL_1 to use this script."
exit 1
fi

git fetch upstream

CHANGED_FILES="$(git diff --diff-filter=ACMRT --name-only upstream/main | grep '\.py$' | tr '\n' ' ')"

if [ "$CHANGED_FILES" != "" ]
then
# Processing files one by one since passing directly $CHANGED_FILES will
# treat the whole variable as a single file.
echo "Running linters ..."
for file in $CHANGED_FILES
do
echo "Checking $file"
usort format "$file"
black "$file" -q
flake8 "$file" || LINT_ERRORS=1
done
else
echo "No changes made to any Python files. Nothing to do."
exit 0
fi

if [ "$LINT_ERRORS" != "" ]
then
echo "One of the linters returned an error. See above output."
# need this so that CI fails
exit 1
fi

# Check if any files were modified by running usort + black
# If so, then the files were formatted incorrectly (e.g. did not pass lint)
CHANGED_FILES="$(git diff --name-only | grep '\.py$' | tr '\n' ' ')"
if [ "$CHANGED_FILES" != "" ]
then
RED="\e[31m"
ENDCOLOR="\e[0m"
echo "------------------------------------------"
echo -e "${RED}[format] These files are not well-formatted:${ENDCOLOR}"
git diff --name-only
echo "------------------------------------------"
echo -e "${RED}[format] Suggested format by lint:${ENDCOLOR}"
git diff
echo "------------------------------------------"
echo "To apply the suggested format, run:"
echo "usort format <file_name>"
echo "black <file_name> -q"
echo "flake8 <file_name>"
echo -e "${RED}You must fix them before merging the pull request.${ENDCOLOR}"
# need this so that CI fails
exit 1
else
echo "All files are well-formatted."
exit 0
fi
lintrunner --force-color
6 changes: 6 additions & 0 deletions scripts/setup_pyre.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash

set -ex

VERSION=$(grep "version" .pyre_configuration | sed -n -e 's/.*\(0\.0\.[0-9]*\).*/\1/p')
pip install pyre-check-nightly==$VERSION
124 changes: 124 additions & 0 deletions tools/linter/adapters/pyre_linter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
import argparse
import concurrent.futures
import json
import logging
import os
import subprocess
import sys
from enum import Enum
from pathlib import Path
from typing import Any, List, NamedTuple, Optional, Set, TypedDict

logger: logging.Logger = logging.getLogger(__name__)


class LintSeverity(str, Enum):
ERROR = "error"
WARNING = "warning"
ADVICE = "advice"
DISABLED = "disabled"


class LintMessage(NamedTuple):
path: Optional[str]
line: Optional[int]
char: Optional[int]
code: str
severity: LintSeverity
name: str
original: Optional[str]
replacement: Optional[str]
description: Optional[str]


class PyreResult(TypedDict):
line: int
column: int
stop_line: int
stop_column: int
path: str
code: int
name: str
description: str
concise_description: str


def run_pyre() -> List[PyreResult]:
proc = subprocess.run(
["pyre", "--output=json", "incremental"],
capture_output=True,
)
return json.loads(proc.stdout)


def check_pyre(
filenames: Set[str],
) -> List[LintMessage]:
try:
results = run_pyre()

return [
LintMessage(
path=result["path"],
line=result["line"],
char=result["column"],
code="pyre",
severity=LintSeverity.WARNING,
name=result["name"],
description=result["description"],
original=None,
replacement=None,
)
for result in results
]
except Exception as err:
return [
LintMessage(
path=None,
line=None,
char=None,
code="pyre",
severity=LintSeverity.ADVICE,
name="command-failed",
original=None,
replacement=None,
description=(f"Failed due to {err.__class__.__name__}:\n{err}"),
)
]


def main() -> None:
parser = argparse.ArgumentParser(
description="Checks files with pyre",
fromfile_prefix_chars="@",
)
parser.add_argument(
"--verbose",
action="store_true",
help="verbose logging",
)
parser.add_argument(
"filenames",
nargs="+",
help="paths to lint",
)
args = parser.parse_args()

logging.basicConfig(
format="<%(processName)s:%(levelname)s> %(message)s",
level=(
logging.NOTSET
if args.verbose
else logging.DEBUG if len(args.filenames) < 1000 else logging.INFO
),
stream=sys.stderr,
)

lint_messages = check_pyre(set(args.filenames))

for lint_message in lint_messages:
print(json.dumps(lint_message._asdict()), flush=True)


if __name__ == "__main__":
main()
Loading

0 comments on commit 2dbfec1

Please sign in to comment.