ChartBoost / ruff-action

A GitHub Action for Ruff
Apache License 2.0
121 stars 21 forks source link

Ruff output in Github Action inconsistent with local Ruff run (`I001`) #20

Closed carschno closed 5 months ago

carschno commented 5 months ago

I have integrated a Ruff linting stage in my Github workflow.

I run Ruff locally on the same branch and in the same version, no issues are detected:

% ruff --version
ruff 0.3.2
% ruff check --verbose --select=I001 document_segmentation/model/page_sequence_tagger.py
[2024-03-13][12:00:16][ruff::resolve][DEBUG] Using configuration file (via parent) at: document-segmentation/pyproject.toml
[2024-03-13][12:00:16][ruff::commands::check][DEBUG] Identified files to lint in: 2.004041ms
[2024-03-13][12:00:16][ruff::commands::check][DEBUG] Checked 1 files in: 18.667µs

No issues found. However, the Github action fails with this error:

Run chartboost/ruff-action@v1
Run if [ "$RUNNER_OS" == "Windows" ]; then
creating virtual environment...
installing ruff from spec 'ruff==0.3.2'...
document_segmentation/model/page_sequence_tagger.py:1:1: I001 [*] Import block is un-sorted or un-formatted
Found 1 error.
[*] 1 fixable with the `--fix` option.
Error: Process completed with exit code 1.

On my code, the issue has only recently started, presumably related to adding a new import statement to the file in question. However, I do not see why Ruff does not detect an issue locally, but does so in the Github action.

The code I have been using is available in the repository, and I have linked the specific actions and files above.

brucearctor commented 5 months ago

@carschno -- is this Correctly finding the issue in the GH Action? I can't help with your local setup, and it reads as if the action is behaving as expected/correctly? If that's the case, can you file the issue with ruff?

To offer at least a place I would look: It looks like your local is picking up your pyproject config, is the GH Action also getting that?

carschno commented 5 months ago

Ah, that was not clear in my description: no, as far as I can see, there is no issue according to the Ruff rules. That is why I suspect an issue related to the Github Action specifically.

This is the code in question (link):

import csv
import logging
import sys
from typing import Any, Optional, TextIO

import torch
import torch.nn as nn
from torch import optim
from torcheval.metrics import (
    Metric,
    MulticlassAccuracy,
    MulticlassF1Score,
    MulticlassPrecision,
    MulticlassRecall,
)
from tqdm import tqdm

import wandb

[...]

The error in the Github action has been triggered since I added import wandb. The options are the same as locally; I have only been able to work around it by adding args: --ignore I001 to the action.

Jendker commented 5 months ago

Same issue here with I001 with GitHub Actions. Problematic imports:

import argparse
import glob
import os
import shutil
import subprocess
import tempfile
import time

import wandb
from my_dashboard.csv_utils import load_csv_to_dict
brucearctor commented 5 months ago

Have you tried fix?

Are you sure your imports are sorted?

carschno commented 5 months ago

Same issue here with I001 with GitHub Actions. Problematic imports:

When I run ruff --fix on those, it sorts them like this:

import argparse
import glob
import os
import shutil
import subprocess
import tempfile
import time

from my_dashboard.csv_utils import load_csv_to_dict

import wandb

So that looks like I001 should actually be triggered, both locally and in the Github action.

brucearctor commented 5 months ago

So, the gh action is working as it should? And, therefore close this? And your issue is with your local ruff not identifying the sane rule breakage?

carschno commented 5 months ago

I was only referring to https://github.com/ChartBoost/ruff-action/issues/20#issuecomment-2007232443, but my initial issue has not changed. The imports that I posted initially have been sorted using ruff --fix, whereas the Github action triggers a I001 error. In https://github.com/ChartBoost/ruff-action/issues/20#issuecomment-2007232443 on the other hand, ruff --fix changes the sorting locally as far as I can see.

brucearctor commented 5 months ago

Not sure I follow ...

import csv
import logging
import sys
from typing import Any, Optional, TextIO

import torch
import torch.nn as nn
import wandb
from torch import optim
from torcheval.metrics import (
    Metric,
    MulticlassAccuracy,
    MulticlassF1Score,
    MulticlassPrecision,
    MulticlassRecall,
)
from tqdm import tqdm

from ..pagexml.datamodel.label import Label
from ..settings import PAGE_SEQUENCE_TAGGER_RNN_CONFIG
from .dataset import DocumentDataset, PageDataset
from .device_module import DeviceModule
from .page_embedding import PageEmbedding

^^ this looks like should be the "correct" sorting, no? And, if it isn't in that order, the expectation is that I001 would fail both locally and in GH action?

carschno commented 5 months ago

Yes, that is exactly the issue. This sorting is the result of running ruff --fix locally, and it passes the ruff check locally. I don't understand why the GitHub action triggers an error.

carschno commented 5 months ago

Ok, I think I figured out the reason. wandb is detected as 1st party package locally:

% poetry run ruff check --fix -v --select=I001 document_segmentation/model/page_sequence_tagger.py
[2024-03-21][10:24:23][ruff::resolve][DEBUG] Using configuration file (via parent) at: .../document-segmentation/pyproject.toml
[2024-03-21][10:24:23][ruff::commands::check][DEBUG] Identified files to lint in: 2.068875ms
[2024-03-21][10:24:23][ruff::diagnostics][DEBUG] Checking: /Users/carstenschnober/LAHTeR/workspace/document-segmentation/document_segmentation/model/page_sequence_tagger.py
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'torch.nn' as Known(ThirdParty) (NoMatch)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'csv' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'sys' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'logging' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'torch' as Known(ThirdParty) (NoMatch)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'wandb' as Known(FirstParty) (SourceMatch("/Users/carstenschnober/LAHTeR/workspace/document-segmentation"))
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'torch' as Known(ThirdParty) (NoMatch)
[2024-03-21][10:24:23][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'torcheval.metrics' as Known(ThirdParty) (NoMatch)

This is because there is a wandb directory in the project root. In a fresh checkout, this directory does not exist. When I remove it locally, the result of ruff --fix looks like in your example.

The wandb directory does not contain Python code, but is created by WandB when it runs. Therefore, this will be a recurring issue whenever wandb library is used. I suppose the underlying issue is hence that Ruff incorrectly detects the wandb folder as a source path; hence not an issue with the Github action.

carschno commented 5 months ago

To clarify on my previous comment: this is issue is not really related to Github Actions, and can therefore be closed here. Before I do that, however, I would be grateful for information on how to debug this properly in Ruff in order to either follow up there, or how to fix this in my configuration.

Jendker commented 5 months ago

Thank you for finding the root cause to the issue @carschno. I run into exact same problem.

In my case I fixed it by adding:

[tool.ruff.lint.isort]
known-third-party = ["wandb"]

to my pyproject.toml configuration. I wish there would be an easy way to debug and diagnose the issue because I'm sure we won't be the last ones confused by it.

brucearctor commented 5 months ago

Why didn't you want to merge: https://github.com/LAHTeR/document_segmentation/pull/56/files ?? the change seemed to fix the issue, and I expect would be fixed locally after merge and after you pulled the remote back to local.

I wonder if this is how you're using git/branches. Both ruff and the action appear to work as expected.

Closing since not an issue with this ruff-action.

carschno commented 5 months ago

Why didn't you want to merge: https://github.com/LAHTeR/document_segmentation/pull/56/files ?? the change seemed to fix the issue, and I expect would be fixed locally after merge and after you pulled the remote back to local.

You are right, I have now merged it into main. The reason was that I am in the process of a large refactoring in another branch, in which this issue is also covered. But it's good to already merge this into main now.

carschno commented 5 months ago

to my pyproject.toml configuration. I wish there would be an easy way to debug and diagnose the issue because I'm sure we won't be the last ones confused by it.

I have created an issue to tackle this: https://github.com/astral-sh/ruff/issues/10519