Skip to content

Commit 2f035d3

Browse files
ko3n1gko3n1g
authored
ci: Run linting per domain (NVIDIA-NeMo#12027)
* ci: Run linting per domain Signed-off-by: oliver könig <okoenig@nvidia.com> * add codeowners Signed-off-by: oliver könig <okoenig@nvidia.com> * add speechllm Signed-off-by: oliver könig <okoenig@nvidia.com> --------- Signed-off-by: oliver könig <okoenig@nvidia.com>
1 parent beab6d4 commit 2f035d3

File tree

7 files changed

+165
-143
lines changed

7 files changed

+165
-143
lines changed

‎.flake8.other‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[flake8]
2+
select =
3+
F541, # f-string without any placeholders
4+
F841, # local variable 'x' is assigned to but never used
5+
F401, # 'x' imported but unused
6+
E741, # ambiguous variable name 'l'
7+
F821, # undefined name 'x'
8+
E266, # too many leading '#' for block comment

‎.flake8.speech‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[flake8]
2+
select =
3+
F541, # f-string without any placeholders
4+
F841, # local variable 'x' is assigned to but never used
5+
F401, # 'x' imported but unused
6+
E741, # ambiguous variable name 'l'
7+
F821, # undefined name 'x'
8+
E266, # too many leading '#' for block comment

‎.github/CODEOWNERS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
.github/ @pablo-garay @ko3n1g @thomasdhc @chtruong814
22
Dockerfile.ci @pablo-garay @ko3n1g @thomasdhc @chtruong814
3+
.pylintrc.* @pablo-garay @ko3n1g @thomasdhc @chtruong814
4+
.flake8.* @pablo-garay @ko3n1g @thomasdhc @chtruong814

‎.github/workflows/code-formatting.yml‎

Lines changed: 1 addition & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: Isort and Black Formatting; PyLint Docs check
1+
name: Isort and Black Formatting
22
# Incrementally reformat only changed files with black, all files with isort
33
#
44
# Replaces pre-commit.ci, since it reformats all the files.
@@ -71,145 +71,3 @@ jobs:
7171
with:
7272
message: Apply isort and black reformatting
7373
commit: --signoff
74-
75-
check_pylint:
76-
name: "check_pylint (strict-mode: ${{ matrix.strict-mode }})"
77-
runs-on: ubuntu-latest
78-
permissions:
79-
contents: write
80-
pull-requests: write
81-
env:
82-
THRESHOLD: 1730937600 # On this date (2024/11/07) we decided to add Pylint. It shall only run in strict mode for files added past this date. For files prior to this date, we will only add a PR comment with PyLint's stdout.
83-
strategy:
84-
matrix:
85-
strict-mode: ["true", "false"]
86-
steps:
87-
- name: Checkout branch
88-
uses: actions/checkout@v4
89-
with:
90-
# setup repository and ref for PRs, see
91-
# https://github.com/EndBug/add-and-commit?tab=readme-ov-file#working-with-prs
92-
repository: ${{ github.event.pull_request.head.repo.full_name }}
93-
ref: ${{ github.event.pull_request.head.ref }}
94-
fetch-depth: 0
95-
96-
# https://github.com/tj-actions/changed-files
97-
- name: Get changed files
98-
id: changed-files
99-
uses: tj-actions/changed-files@v44
100-
with:
101-
files: |
102-
**.py
103-
104-
- name: Setup Python env
105-
uses: actions/setup-python@v5
106-
with:
107-
python-version: "3.10"
108-
109-
- name: pylint
110-
if: ${{ steps.changed-files.outputs.any_changed == 'true' && !contains( github.event.pull_request.labels.*.name, 'skip-docs') }}
111-
id: pylint
112-
env:
113-
# only *.py files included
114-
STRICT_MODE: ${{ matrix.strict-mode }}
115-
CHANGED_FILES: "${{ steps.changed-files.outputs.all_changed_files }}"
116-
run: |
117-
pip install pylint
118-
119-
FILTERED=()
120-
for file in $CHANGED_FILES; do
121-
DATE=$(git log --format=%ad --date=unix "$file" | tail -1)
122-
123-
if [[ "$STRICT_MODE" == "true" ]]; then
124-
if [[ "$DATE" -gt "$THRESHOLD" ]]; then
125-
FILTERED+=("$file")
126-
fi
127-
else
128-
if [[ "$DATE" -le "$THRESHOLD" ]]; then
129-
FILTERED+=("$file")
130-
fi
131-
fi
132-
done
133-
134-
if [ ${#FILTERED[@]} -eq 0 ]; then
135-
echo "No files to check."
136-
exit 0
137-
fi
138-
139-
echo "Will run on these files:
140-
${FILTERED[@]}"
141-
142-
set +e
143-
LOG=$(pylint ${FILTERED[@]})
144-
EXIT_CODE=$?
145-
set -e
146-
147-
set +x
148-
echo "OUTPUT<<EOF" >> $GITHUB_ENV
149-
echo "$LOG" >> $GITHUB_ENV
150-
echo "EOF" >> $GITHUB_ENV
151-
echo "log=$LOG"
152-
set -x
153-
154-
echo "exit-code=$EXIT_CODE" | tee -a "$GITHUB_OUTPUT"
155-
156-
if [[ "${{ matrix.strict-mode }}" == "true" ]]; then
157-
HEADER="🚨 The following files must be fixed before merge!"
158-
else
159-
HEADER="🙏 The following files have warnings. In case you are familiar with these, please try helping us to improve the code base."
160-
fi
161-
echo "header=$HEADER" | tee -a "$GITHUB_OUTPUT"
162-
163-
exit $([[ "$EXIT_CODE" -ne 0 && "$STRICT_MODE" == "true" ]] && echo $EXIT_CODE || echo 0)
164-
165-
- name: Find Comment
166-
if: ${{ always() }}
167-
uses: peter-evans/find-comment@v3
168-
id: fc
169-
with:
170-
issue-number: ${{ github.event.number }}
171-
body-includes: <!-- pylint-output-strict-mode-${{ matrix.strict-mode }} -->
172-
173-
- name: Delete comment
174-
if: ${{ always() && steps.fc.outputs.comment-id != '' }}
175-
env:
176-
GH_TOKEN: ${{ secrets.github_token }}
177-
REPOSITORY: ${{ github.repository }}
178-
COMMENT_ID: ${{ steps.fc.outputs.comment-id }}
179-
run: |
180-
curl -L \
181-
-X DELETE \
182-
-H "Accept: application/vnd.github+json" \
183-
-H "Authorization: Bearer $GH_TOKEN" \
184-
-H "X-GitHub-Api-Version: 2022-11-28" \
185-
https://api.github.com/repos/$REPOSITORY/issues/comments/$COMMENT_ID
186-
187-
- name: Add PR comment for PyLint
188-
if: ${{ always() && steps.pylint.outputs.exit-code != '0' }}
189-
uses: peter-evans/create-or-update-comment@v4
190-
with:
191-
issue-number: ${{ github.event.number }}
192-
body: |
193-
<!-- pylint-output-strict-mode-${{ matrix.strict-mode }} -->
194-
195-
beep boop 🤖: ${{ steps.pylint.outputs.header }}
196-
197-
---
198-
199-
Your code was analyzed with PyLint. The following annotations have been identified:
200-
201-
```
202-
${{ env.OUTPUT }}
203-
```
204-
205-
---
206-
207-
Mitigation guide:
208-
209-
* Add sensible and useful docstrings to functions and methods
210-
* For trivial methods like getter/setters, consider adding `# pylint: disable=C0116` inside the function itself
211-
* To disable multiple functions/methods at once, put a `# pylint: disable=C0116` before the first and a `# pylint: enable=C0116` after the last.
212-
213-
By applying these rules, we reduce the occurance of this message in future.
214-
215-
Thank you for improving NeMo's documentation!

‎.github/workflows/code-linting.yml‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
name: PyLint and flake8 linting
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- '**.py'
7+
types: [ opened, synchronize, reopened, labeled, unlabeled ]
8+
9+
jobs:
10+
linting:
11+
name: 'Domain: ${{ matrix.domain }}'
12+
runs-on: ubuntu-latest
13+
strategy:
14+
fail-fast: false
15+
matrix:
16+
domain: [speech, other]
17+
env:
18+
DOMAIN: ${{ matrix.domain }}
19+
steps:
20+
- name: Checkout
21+
uses: actions/checkout@v4
22+
23+
- name: Select filter
24+
id: filter
25+
run: |
26+
if [[ "$DOMAIN" == "speech" ]]; then
27+
FILTER=$(jq -crn '[
28+
"nemo/collections/asr/**",
29+
"nemo/collections/tts/**",
30+
"nemo/collections/audio/**",
31+
"nemo/collections/multimodal/speech_llm/**",
32+
"nemo/collections/speechlm/**"
33+
] | join(",")')
34+
35+
else
36+
FILTER=$(jq -crn '[
37+
"nemo/**",
38+
"!nemo/collections/asr/**",
39+
"!nemo/collections/tts/**",
40+
"!nemo/collections/audio/**",
41+
"!nemo/collections/multimodal/speech_llm/**",
42+
"!nemo/collections/speechlm/**"
43+
] | join(",")')
44+
fi
45+
46+
echo "main=$FILTER" | tee -a "$GITHUB_OUTPUT"
47+
48+
- name: Get changed files
49+
id: changed-files
50+
uses: tj-actions/changed-files@v44
51+
with:
52+
files: ${{ steps.filter.outputs.main }}
53+
files_separator: ","
54+
separator: " "
55+
56+
- name: Run PyLint
57+
id: pylint
58+
env:
59+
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
60+
run: |
61+
if [[ -z "$CHANGED_FILES" ]]; then
62+
echo Nothing to lint.
63+
echo "exit-code=0" | tee -a "$GITHUB_OUTPUT"
64+
exit 0
65+
fi
66+
67+
pip install pylint
68+
set +e
69+
pylint --output "pylintrc.$DOMAIN.txt" --rcfile ".pylintrc.$DOMAIN" ${CHANGED_FILES[@]}
70+
echo "exit-code=$?" | tee -a "$GITHUB_OUTPUT"
71+
72+
- name: Run flake8
73+
id: flake8
74+
env:
75+
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
76+
run: |
77+
if [[ -z "$CHANGED_FILES" ]]; then
78+
echo Nothing to lint.
79+
echo "exit-code=0" | tee -a "$GITHUB_OUTPUT"
80+
exit 0
81+
fi
82+
83+
pip install flake8
84+
set +e
85+
flake8 --output "flake8.$DOMAIN.txt" --config ".flake8.$DOMAIN" ${CHANGED_FILES[@]}
86+
echo "exit-code=$?" | tee -a "$GITHUB_OUTPUT"
87+
88+
- name: Summary
89+
env:
90+
PYLINT: ${{ steps.pylint.outputs.exit-code == 0 }}
91+
FLAKE8: ${{ steps.flake8.outputs.exit-code == 0 }}
92+
run: |
93+
94+
if [[ "$PYLINT" != "true" ]]; then
95+
echo "Pylint output:" | tee -a $GITHUB_STEP_SUMMARY
96+
97+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
98+
cat pylintrc.$DOMAIN.txt | tee -a $GITHUB_STEP_SUMMARY
99+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
100+
fi
101+
102+
if [[ "$FLAKE8" != "true" ]]; then
103+
echo "Flake8 output:" | tee -a $GITHUB_STEP_SUMMARY
104+
105+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
106+
cat flake8.$DOMAIN.txt | tee -a $GITHUB_STEP_SUMMARY
107+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
108+
fi
109+
110+
if [[ "$PYLINT" != "true" || "$FLAKE8" != "true" ]]; then
111+
echo "The following directories got scanned:" | tee -a $GITHUB_STEP_SUMMARY
112+
113+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
114+
echo ${{ steps.filter.outputs.main }} | tee -a $GITHUB_STEP_SUMMARY
115+
echo '```' | tee -a $GITHUB_STEP_SUMMARY
116+
117+
exit 1
118+
fi
119+
120+
Nemo_Linting_Test:
121+
needs: linting
122+
runs-on: ubuntu-latest
123+
if: always()
124+
steps:
125+
- name: Main
126+
env:
127+
RESULTS: ${{ toJson(needs.linting) }}
128+
run: |
129+
RESULT=$(echo "$RESULTS" | jq -r '.result')
130+
131+
if [[ "$RESULT" == "success" ]]; then
132+
echo "All passed."
133+
exit 0
134+
else
135+
echo "Some linting domains failed."
136+
exit 1
137+
fi

.pylintrc renamed to .pylintrc.other

File renamed without changes.

‎.pylintrc.speech‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[MAIN]
2+
ignore-paths=tests
3+
max-line-length=119
4+
5+
[MESSAGES CONTROL]
6+
disable=all
7+
8+
enable=W0611
9+
# W0611: unused-import

0 commit comments

Comments
 (0)