Repository: scikit-learn/scikit-learn
Pull request: MNT linter bot: tool versions and finished linting check
This fixes a few issues:

- linter should fail if downloading `linting.sh` fails (hence moving `set +e`)
- linter should fail if `linting.sh` failes to finish (@glemaitre noticed this in some PRs)
- comment includes the version of the tool used (@lucyleeow, Fixes https://github.com/scikit-learn/scikit-learn/issues/26676)

cc @thomasjpfan 

example run: https://github.com/adrinjalali/gh-action-test/pull/7#issuecomment-1604021218

Problem statement:
CI: Add black version in linter bot
Great job on the linter bot @adrinjalali !

Would it be a good idea to add in the comment what the desired black (and ruff ??) versions are? I think I recall for black at least that different versions can affect formatting...

Hints from issue discussion:

Commit message: MNT linter bot: tool versions and finished linting check

Patch to review:
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index e4b606f36b7f8..9648f66d840d4 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -34,14 +34,19 @@ jobs:
           source build_tools/shared.sh
           # Include pytest compatibility with mypy
           pip install pytest ruff $(get_dep mypy min) $(get_dep black min) cython-lint
+          # we save the versions of the linters to be used in the error message later.
+          python -c "from importlib.metadata import version; print(f\"ruff={version('ruff')}\")" >> /tmp/versions.txt
+          python -c "from importlib.metadata import version; print(f\"mypy={version('mypy')}\")" >> /tmp/versions.txt
+          python -c "from importlib.metadata import version; print(f\"black={version('black')}\")" >> /tmp/versions.txt
+          python -c "from importlib.metadata import version; print(f\"cython-lint={version('cython-lint')}\")" >> /tmp/versions.txt
 
       - name: Run linting
         id: lint-script
         # We download the linting script from main, since this workflow is run
         # from main itself.
         run: |
+          curl https://raw.githubusercontent.com/${{ github.repository }}/main/build_tools/linting.sh --retry 5 -o ./build_tools/linting.sh
           set +e
-          curl https://raw.githubusercontent.com/scikit-learn/scikit-learn/main/build_tools/linting.sh --retry 5 -o ./build_tools/linting.sh
           ./build_tools/linting.sh &> /tmp/linting_output.txt
           cat /tmp/linting_output.txt
 
@@ -50,7 +55,9 @@ jobs:
         uses: actions/upload-artifact@v3
         with:
           name: lint-log
-          path: /tmp/linting_output.txt
+          path: |
+            /tmp/linting_output.txt
+            /tmp/versions.txt
           retention-days: 1
 
   comment:
@@ -92,4 +99,5 @@ jobs:
           BRANCH_SHA: ${{ github.event.pull_request.head.sha }}
           RUN_ID: ${{ github.run_id }}
           LOG_FILE: linting_output.txt
+          VERSIONS_FILE: versions.txt
         run: python ./build_tools/get_comment.py
diff --git a/build_tools/get_comment.py b/build_tools/get_comment.py
index 143227051eca8..f4544b80deadc 100644
--- a/build_tools/get_comment.py
+++ b/build_tools/get_comment.py
@@ -7,6 +7,26 @@
 import requests
 
 
+def get_versions(versions_file):
+    """Get the versions of the packages used in the linter job.
+
+    Parameters
+    ----------
+    versions_file : str
+        The path to the file that contains the versions of the packages.
+
+    Returns
+    -------
+    versions : dict
+        A dictionary with the versions of the packages.
+    """
+    with open(versions_file, "r") as f:
+        versions = f.read().splitlines()
+    versions = [v.split("=") for v in versions]
+    versions = {v[0]: v[1] for v in versions}
+    return versions
+
+
 def get_step_message(log, start, end, title, message, details):
     """Get the message for a specific test.
 
@@ -52,10 +72,29 @@ def get_step_message(log, start, end, title, message, details):
     return res
 
 
-def get_message(log_file, repo, pr_number, sha, run_id, details):
+def get_message(log_file, repo, pr_number, sha, run_id, details, versions):
     with open(log_file, "r") as f:
         log = f.read()
 
+    sub_text = (
+        "\n\n<sub> _Generated for commit:"
+        f" [{sha[:7]}](https://github.com/{repo}/pull/{pr_number}/commits/{sha}). "
+        "Link to the linter CI: [here]"
+        f"(https://github.com/{repo}/actions/runs/{run_id})_ </sub>"
+    )
+
+    if "### Linting completed ###" not in log:
+        return (
+            "## ❌ Linting issues\n\n"
+            "There was an issue running the linter job. Please update with "
+            "`upstream/main` ([link]("
+            "https://scikit-learn.org/dev/developers/contributing.html"
+            "#how-to-contribute)) and push the changes. If you already have done "
+            "that, please send an empty commit with `git commit --allow-empty` "
+            "and push the changes to trigger the CI.\n\n"
+            + sub_text
+        )
+
     message = ""
 
     # black
@@ -68,7 +107,8 @@ def get_message(log_file, repo, pr_number, sha, run_id, details):
             "`black` detected issues. Please run `black .` locally and push "
             "the changes. Here you can see the detected issues. Note that "
             "running black might also fix some of the issues which might be "
-            "detected by `ruff`."
+            "detected by `ruff`. Note that the installed `black` version is "
+            f"`black={versions['black']}`."
         ),
         details=details,
     )
@@ -82,7 +122,8 @@ def get_message(log_file, repo, pr_number, sha, run_id, details):
         message=(
             "`ruff` detected issues. Please run `ruff --fix --show-source .` "
             "locally, fix the remaining issues, and push the changes. "
-            "Here you can see the detected issues."
+            "Here you can see the detected issues. Note that the installed "
+            f"`ruff` version is `ruff={versions['ruff']}`."
         ),
         details=details,
     )
@@ -95,7 +136,8 @@ def get_message(log_file, repo, pr_number, sha, run_id, details):
         title="`mypy`",
         message=(
             "`mypy` detected issues. Please fix them locally and push the changes. "
-            "Here you can see the detected issues."
+            "Here you can see the detected issues. Note that the installed `mypy` "
+            f"version is `mypy={versions['mypy']}`."
         ),
         details=details,
     )
@@ -108,7 +150,9 @@ def get_message(log_file, repo, pr_number, sha, run_id, details):
         title="`cython-lint`",
         message=(
             "`cython-lint` detected issues. Please fix them locally and push "
-            "the changes. Here you can see the detected issues."
+            "the changes. Here you can see the detected issues. Note that the "
+            "installed `cython-lint` version is "
+            f"`cython-lint={versions['cython-lint']}`."
         ),
         details=details,
     )
@@ -152,13 +196,6 @@ def get_message(log_file, repo, pr_number, sha, run_id, details):
         details=details,
     )
 
-    sub_text = (
-        "\n\n<sub> _Generated for commit:"
-        f" [{sha[:7]}](https://github.com/{repo}/pull/{pr_number}/commits/{sha}). "
-        "Link to the linter CI: [here]"
-        f"(https://github.com/{repo}/actions/runs/{run_id})_ </sub>"
-    )
-
     if not message:
         # no issues detected, so this script "fails"
         return (
@@ -215,10 +252,8 @@ def find_lint_bot_comments(repo, token, pr_number):
     response.raise_for_status()
     all_comments = response.json()
 
-    failed_comment = "This PR is introducing linting issues. Here's a summary of the"
-    success_comment = (
-        "All linting checks passed. Your pull request is in excellent shape"
-    )
+    failed_comment = "❌ Linting issues"
+    success_comment = "✔️ Linting Passed"
 
     # Find all comments that match the linting bot, and return the first one.
     # There should always be only one such comment, or none, if the PR is
@@ -269,6 +304,9 @@ def create_or_update_comment(comment, message, repo, pr_number, token):
     sha = os.environ["BRANCH_SHA"]
     log_file = os.environ["LOG_FILE"]
     run_id = os.environ["RUN_ID"]
+    versions_file = os.environ["VERSIONS_FILE"]
+
+    versions = get_versions(versions_file)
 
     if not repo or not token or not pr_number or not log_file or not run_id:
         raise ValueError(
@@ -290,6 +328,7 @@ def create_or_update_comment(comment, message, repo, pr_number, token):
             sha=sha,
             run_id=run_id,
             details=True,
+            versions=versions,
         )
         create_or_update_comment(
             comment=comment,
@@ -309,6 +348,7 @@ def create_or_update_comment(comment, message, repo, pr_number, token):
             sha=sha,
             run_id=run_id,
             details=False,
+            versions=versions,
         )
         create_or_update_comment(
             comment=comment,
diff --git a/build_tools/linting.sh b/build_tools/linting.sh
index 0344f8e4f53b2..28d16a8bbed32 100755
--- a/build_tools/linting.sh
+++ b/build_tools/linting.sh
@@ -113,6 +113,8 @@ else
     global_status=1
 fi
 
+echo -e "### Linting completed ###\n"
+
 if [[ $global_status -eq 1 ]]
 then
     echo -e "Linting failed\n"


Code review comments:
diff_hunk: @@ -7,6 +7,26 @@
 import requests
 
 
+def get_versions(versions_file):
+    """Get the versions of the packages used in the linter job.
+
+    Parameters
+    ----------
+    versions_file : str
+        The path to the file that contains the versions of the packages.
+
+    Returns
+    -------
+    versions : dict
+        A dictionary with the versions of the packages.
+    """
+    with open(versions_file, "r") as f:
+        versions = f.read().splitlines()
+    versions = [v.split("=") for v in versions]
+    versions = {v[0]: v[1] for v in versions}
path: build_tools/get_comment.py
text: @user1:
```python
with open("versions.txt", "r") as f:
    versions = dict(line.strip().split("=") for line in f)
```