[MNT] move linting to ruff, resolve strange linting behaviour

[fkiraly]

We should move the linting to ruff - good first issue for a maintenance affine contributor.

There are also some strange behaviours in the precommits which I cannot pinpoint, e.g., the linter insisting on long single lines instead of line breaks where they would be appropriate. Though I would hope this is resolved as a side effect of moving to ruff.

[airookie17]

Hello @fkiraly!

I'm interested in working on this issue and have started setting up ruff to replace the existing linters. Could you please assign this issue to me?

Thank you!

[fkiraly]

Of course! You can look at @fnhirwa's work in sktime in case you need a template.

[airookie17]

Hello @fkiraly! I'm working on replacing the current linting tools with Ruff. However, I've encountered a situation I'd like to clarify:

I ran the pre-commit hooks using pre-commit run --all-files with the current setup, and all checks passed without showing the "strange behaviors" mentioned in the issue description. I couldn't identify specific instances where the linter is "insisting on long single lines instead of line breaks."

Could you give me more specific examples of the issues you've had? This would help ensure that the new ruff configuration addresses these problems effectively.

In the meantime, I'll go ahead and set up ruff as the new linter. Once implemented, I'll test it thoroughly across the codebase and report back with the results.

Thanks!

[fkiraly]

I couldn't identify specific instances where the linter is "insisting on long single lines instead of line breaks."

That is because all lines are already compliant with the current linting - many files in the code base include lines well in excess of the 79 or even larger bounds on number of characters.

Further, if one tries to add line breaks, the linter resets them to "one line code".

[airookie17]

Hi @fkiraly ,

I've updated the project to use Ruff for linting, replacing Flake8 and isort. While making this transition, I've encountered some issues that I'd like your help with.

Issues Encountered

1. Linting Errors Requiring Manual Fixes

   After running Ruff, several linting errors are being flagged that require manual intervention:

   • Unnecessary List Comprehensions (C416, C419): Ruff suggests that some list comprehensions can be simplified or rewritten as generator expressions or removed entirely.
   • Security Warnings (S301, S310): These relate to the use of pickle.loads() and urlretrieve(), which Ruff flags as potential security risks.

   Proposed Solutions

   • Option 1: Use --unsafe-fixes with Ruff to automatically fix the issues, followed by a careful review of the changes to ensure no unintended side effects.
   • Option 2: Add the specific error codes to Ruff's extend-ignore list in the pyproject.toml to suppress these warnings for now. The codes are C416, C419, S301, S310.

[fkiraly]

I've updated the project to use Ruff for linting, replacing Flake8 and isort

Thanks, great!

Could you make a pull request with these changes? If you are not done yet, you can make a draft pull request.

Regarding the linting errors, I would not add blanket exceptions to these, and also not fix them automatically in the first PR.

I would configure ruff so only changed files are linted, and go through linting errors in a separate PR. Here, we could apply case-by-case fixes, or exceptions (but no blanket exceptions).

[airookie17]

I've created a draft PR (#1692) that replaces flake8 and isort with ruff

Regarding the linting errors, I would not add blanket exceptions to these, and also not fix them automatically in the first PR.

As mentioned I haven't added any exceptions for the warnings/errors occurring.

Also, I've noticed that ruff can also be used for formatting. So I would like to know whether I should try replacing black with ruff as well in the same PR or just stick to replacing linters for now?

Thanks!

[fkiraly]

Also, I've noticed that ruff can also be used for formatting. So I would like to know whether I should try replacing black with ruff as well in the same PR or just stick to replacing linters for now?

The formatting might trigger a lot of changes, so it might be worth adding black-equivalent functionality in a separate PR - noting that the repository currently does not seem to be black formatted.

[airookie17]

Hi @fkiraly, I've marked my PR to be reviewed now.

Notes:

• Existing linting rules have been kept as-is, just translated to configuration format for ruff .
• I have not added blanket exceptions for the new error types detected by ruff.
• No changes to formatting have been made either.

New Linting Errors Detected by Ruff:

• C416: Unnecessary use of list comprehension - rewrite using list()
• C419: Unnecessary list comprehension
• S101: Use of assert detected
• S301: pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue
• S310: Audit url open for permitted schemes. Allowing use of file: or custom schemes is often unexpected

Please let me know if any adjustments are needed. Thanks!

[fkiraly]

Looks great! Seems to be 1:1 translation. I left a comment there.