Skip to content

Linting and formatting: add Ruff #1000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 64 commits into from
Jun 10, 2025
Merged

Linting and formatting: add Ruff #1000

merged 64 commits into from
Jun 10, 2025

Conversation

Laurynas-Jagutis
Copy link
Member

Adds Ruff linting and formatting, replaces the old linting and formatting tools.

@Laurynas-Jagutis Laurynas-Jagutis self-assigned this May 20, 2025
@Laurynas-Jagutis Laurynas-Jagutis added the feature New feature or request label May 20, 2025
@Laurynas-Jagutis Laurynas-Jagutis marked this pull request as ready for review May 27, 2025 04:50
Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although not too bad, it is quite a lot of files. I propose the following:
create a separate branch off main in which you do the github workflows, precommit and pyproject.toml changes
this branch should not contain any code changes.
instead, you can disable any violating rules in the pyproject.toml in the configuration
then rebase this branch off the other one
note: this may cause some merge/rebase conflicts but after merging, this branch in your local, rebased checkout should be completely the same as the one in the PR ( git diff origin/feature/add-ruff yields an empty diff)
all the configuration disablings in the pyproject.toml should now be "normal" again
then this change is split into 2 stages which helps people migrate, as well as prove that this is indeed the right way to go forward

Copy link
Member

@TonyXiang8787 TonyXiang8787 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to change the recommended vscode extensions to ruff.

Copy link
Contributor

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have ruff now.

Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
Signed-off-by: Laurynas Jagutis <[email protected]>
@mgovers mgovers force-pushed the feature/add-ruff branch from 171e8e3 to f5a7d70 Compare June 4, 2025 13:49
Copy link
Contributor

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one minor additional thing left from me. There are still some pylint comments at dataset_class_maps.py.jinja. Remove those as well and don't forget to run the code generator afterwards. Besides that, it looks good to me and locally it is formatted the same as here.

@mgovers
Copy link
Member

mgovers commented Jun 5, 2025

Quality Gate Passed Quality Gate passed

Issues 1 New issue 0 Accepted issues

Measures 0 Security Hotspots 96.7% Coverage on New Code 0.0% Duplication on New Code

See analysis details on SonarQube Cloud

please just add a nosonar because it's intended

@Laurynas-Jagutis Laurynas-Jagutis changed the base branch from main to fix/mypy-version June 10, 2025 12:22
Base automatically changed from fix/mypy-version to main June 10, 2025 12:45
Copy link

Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many previously disabled rules were removed because we don't have PL checks enabled. Some of these may be re-added in a later PR, but for now, let's go ahead and merge

@mgovers mgovers enabled auto-merge June 10, 2025 13:02
@mgovers mgovers added this pull request to the merge queue Jun 10, 2025
Merged via the queue into main with commit 78cd19a Jun 10, 2025
31 checks passed
@mgovers mgovers deleted the feature/add-ruff branch June 10, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants