Skip to content

Commit e9b378e

Browse files
authored
[Mypy] Add type check with mypy and fix storage (skypilot-org#1519)
* Add mypy for sky/data/storage.py * not implemented function * Fix format * fix * pyproject * fix * switch to python 3.8 * add requirements-dev.txt * fix format * ignore imports * fix handle * fix handle * use mypy file * Fix * yapf
1 parent a0760df commit e9b378e

File tree

10 files changed

+136
-49
lines changed

10 files changed

+136
-49
lines changed

.github/workflows/mypy-generic.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# This is needed for GitHub Actions for the "Waiting for status to be reported" problem,
2+
# according to https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks
3+
name: mypy
4+
5+
on:
6+
# Trigger the workflow on push or pull request,
7+
# but only for the main branch
8+
push:
9+
branches:
10+
- master
11+
- 'releases/**'
12+
pull_request:
13+
branches:
14+
- master
15+
- 'releases/**'
16+
jobs:
17+
mypy:
18+
# Need to specify 20.04, because ubuntu-latest does not work with
19+
# python 3.6: https://github.com/actions/setup-python/issues/355#issuecomment-1335042510
20+
runs-on: ubuntu-20.04
21+
steps:
22+
- run: 'echo "No mypy to run"'

.github/workflows/mypy.yml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
name: mypy
2+
3+
on:
4+
# Trigger the workflow on push or pull request,
5+
# but only for the main branch
6+
push:
7+
branches:
8+
- master
9+
- 'releases/**'
10+
pull_request:
11+
branches:
12+
- master
13+
- 'releases/**'
14+
jobs:
15+
mypy:
16+
# Need to specify 20.04, because ubuntu-latest does not work with
17+
# python 3.6: https://github.com/actions/setup-python/issues/355#issuecomment-1335042510
18+
runs-on: ubuntu-20.04
19+
strategy:
20+
matrix:
21+
python-version: ["3.8"]
22+
steps:
23+
- uses: actions/checkout@v2
24+
- name: Set up Python ${{ matrix.python-version }}
25+
uses: actions/setup-python@v2
26+
with:
27+
python-version: ${{ matrix.python-version }}
28+
- name: Install dependencies
29+
run: |
30+
python -m pip install --upgrade pip
31+
pip install mypy==$(grep mypy requirements-dev.txt | cut -d'=' -f3)
32+
- name: Running mypy
33+
run: |
34+
mypy @tests/mypy_files.txt

format.sh

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ builtin cd "$ROOT" || exit 1
2424
YAPF_VERSION=$(yapf --version | awk '{print $2}')
2525
PYLINT_VERSION=$(pylint --version | head -n 1 | awk '{print $2}')
2626
PYLINT_QUOTES_VERSION=$(pip list | grep pylint-quotes | awk '{print $2}')
27+
MYPY_VERSION=$(mypy --version | awk '{print $2}')
2728

2829
# # params: tool name, tool version, required version
2930
tool_version_check() {
@@ -33,9 +34,10 @@ tool_version_check() {
3334
fi
3435
}
3536

36-
tool_version_check "yapf" $YAPF_VERSION "0.32.0"
37-
tool_version_check "pylint" $PYLINT_VERSION "2.8.2"
38-
tool_version_check "pylint-quotes" $PYLINT_QUOTES_VERSION "0.2.3"
37+
tool_version_check "yapf" $YAPF_VERSION "$(grep yapf requirements-dev.txt | cut -d'=' -f3)"
38+
tool_version_check "pylint" $PYLINT_VERSION "$(grep "pylint==" requirements-dev.txt | cut -d'=' -f3)"
39+
tool_version_check "pylint-quotes" $PYLINT_QUOTES_VERSION "$(grep "pylint-quotes==" requirements-dev.txt | cut -d'=' -f3)"
40+
tool_version_check "mypy" "$MYPY_VERSION" "$(grep mypy requirements-dev.txt | cut -d'=' -f3)"
3941

4042
YAPF_FLAGS=(
4143
'--recursive'
@@ -87,6 +89,12 @@ else
8789
format_changed
8890
fi
8991

92+
# Run mypy
93+
# TODO(zhwu): When more of the codebase is typed properly, the mypy flags
94+
# should be set to do a more stringent check.
95+
echo 'SkyPilot mypy:'
96+
mypy @tests/mypy_files.txt
97+
9098
# Run Pylint
9199
echo 'Sky Pylint:'
92100
pylint --load-plugins pylint_quotes sky

pyproject.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,8 @@ env = [
1616
"SKYPILOT_DEBUG=1",
1717
"SKYPILOT_DISABLE_USAGE_COLLECTION=1"
1818
]
19+
20+
[tool.mypy]
21+
python_version = "3.8"
22+
follow_imports = "skip"
23+
ignore_missing_imports = true

requirements-dev.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ pylint==2.8.2
55
pylint-quotes==0.2.3
66
toml==0.10.2
77

8+
# type checking
9+
mypy==0.991
10+
811
# testing
912
pytest
1013
pytest-xdist

sky/adaptors/azure.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def wrapper(*args, **kwargs):
1313
global azure
1414
if azure is None:
1515
try:
16-
import azure as _azure
16+
import azure as _azure # type: ignore
1717
azure = _azure
1818
except ImportError:
1919
raise ImportError('Fail to import dependencies for Azure.'

sky/data/data_utils.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from multiprocessing import pool
44
import os
55
import subprocess
6-
from pathlib import Path
76
from typing import Any, Callable, Dict, List, Optional, Tuple
87
import urllib.parse
98

@@ -110,7 +109,7 @@ def _group_files_by_dir(
110109
return grouped_files, dirs
111110

112111

113-
def parallel_upload(source_path_list: List[Path],
112+
def parallel_upload(source_path_list: List[str],
114113
filesync_command_generator: Callable[[str, List[str]], str],
115114
dirsync_command_generator: Callable[[str, str], str],
116115
bucket_name: str,

0 commit comments

Comments
 (0)