Skip to content

[MNT] move pre-commit config to ruff#508

Open
arnavk23 wants to merge 15 commits intosktime:mainfrom
arnavk23:fix/pre-commit-ruff
Open

[MNT] move pre-commit config to ruff#508
arnavk23 wants to merge 15 commits intosktime:mainfrom
arnavk23:fix/pre-commit-ruff

Conversation

@arnavk23
Copy link
Copy Markdown
Contributor

@arnavk23 arnavk23 commented Mar 2, 2026

Reference Issues/PRs

Fixes #502

What does this implement/fix? Explain your changes.

This PR migrates the pre-commit configuration to use ruff as the unified tool for code formatting and linting, replacing the separate isort, black, and flake8 hooks. This aligns skbase with sktime and other GC.OS repositories for consistency and performance.

Does your contribution introduce a new dependency? If yes, which one?

Yes, ruff is added as a new dependency in the linters optional dependencies group in pyproject.toml. This is necessary to replace the functionality of isort, black, and flake8, providing a single, efficient tool for linting and formatting.

What should a reviewer concentrate their feedback on?

  • Correctness of the ruff configuration in pyproject.toml (rules, ignores, excludes).
  • Proper setup of pre-commit hooks in .pre-commit-config.yaml

Any other comments?

PR checklist

For all contributions
  • I've reviewed the project documentation on contributing
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [CI/CD], [MNT], [DOC], or [BUG] indicating whether
    the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions
  • Unit tests have been added covering code functionality
  • Appropriate docstrings have been added (see documentation standards)
  • New public functionality has been added to the API Reference

arnavk23 added 4 commits March 2, 2026 15:50
- Replace isort, black, flake8 with ruff in pre-commit hooks
- Add ruff configuration to pyproject.toml
- Update linters dependencies
- Format and fix code with ruff
@arnavk23 arnavk23 force-pushed the fix/pre-commit-ruff branch from 40df124 to ac62a6d Compare March 2, 2026 11:42
@arnavk23 arnavk23 marked this pull request as draft March 2, 2026 12:00
@arnavk23
Copy link
Copy Markdown
Contributor Author

arnavk23 commented Mar 2, 2026

@fkiraly Please review this pr. Focus only on the precommit file as the rest changes are caused by moving to ruff.

@arnavk23 arnavk23 marked this pull request as ready for review March 2, 2026 17:36
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 79.74138% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.73%. Comparing base (306958d) to head (7928ffa).
⚠️ Report is 191 commits behind head on main.

Files with missing lines Patch % Lines
skbase/testing/test_all_objects.py 48.00% 13 Missing ⚠️
skbase/base/_meta.py 29.41% 12 Missing ⚠️
skbase/utils/deep_equals/_deep_equals.py 60.00% 12 Missing ⚠️
skbase/base/_base.py 86.36% 3 Missing ⚠️
skbase/testing/utils/_conditional_fixtures.py 66.66% 2 Missing ⚠️
skbase/utils/dependencies/_import.py 0.00% 2 Missing ⚠️
skbase/base/_clone_plugins.py 75.00% 1 Missing ⚠️
skbase/utils/dependencies/_dependencies.py 87.50% 1 Missing ⚠️
skbase/utils/git_diff.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #508      +/-   ##
==========================================
- Coverage   85.07%   83.73%   -1.35%     
==========================================
  Files          45       52       +7     
  Lines        3015     3854     +839     
==========================================
+ Hits         2565     3227     +662     
- Misses        450      627     +177     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fkiraly fkiraly added the maintenance Continuous integration, unit testing & package distribution label Mar 5, 2026
@fkiraly fkiraly changed the title MNT: move pre-commit config to ruff [MNT] move pre-commit config to ruff Mar 5, 2026
@arnavk23
Copy link
Copy Markdown
Contributor Author

arnavk23 commented Apr 9, 2026

@fkiraly Franz I think this change can be added as it is completely done and whenever you push a pr here, I have to clear merge conflicts.

Comment thread pyproject.toml
[tool.bandit]
exclude_dirs = ["*/tests/*", "*/testing/*"]

[tool.setuptools]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why do we remove the setuptools config - seems like an accident

Copy link
Copy Markdown
Contributor Author

@arnavk23 arnavk23 Apr 12, 2026

Choose a reason for hiding this comment

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

Thank you Franz for pointing that out. I have fixed it. You can review the pr again and hopefully merge this.

Also there are 3 more prs on this repo which you can check out.

Copy link
Copy Markdown
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Thanks! I think there is a minor issue here, see above. Please check.

Copy link
Copy Markdown
Contributor

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

@arnavk23, since you are using a lot of AI in your pull requests - and often not well checked - can you kindly explain explicitly how you applied the formatting to all files?

Further, how did you select the configuration? E.g., did you copy it from somewhere, or is the list of excepted codes coming out of an AI?

@arnavk23
Copy link
Copy Markdown
Contributor Author

arnavk23 commented Apr 19, 2026

@fkiraly I think there may be a misunderstanding. Most file changes came from Ruff auto-formatting during the migration. I followed the project’s existing Black/Flake8 behavior and put that into Ruff rather than introducing a new style. And the ignore are for warnings and error that may arise due to moving the previous files to ruff.

@fkiraly
Copy link
Copy Markdown
Contributor

fkiraly commented Apr 19, 2026

sure - can you please let me know how exactly you applied ruff?

Also please comment on the question where the excludes are coming from.

@arnavk23
Copy link
Copy Markdown
Contributor Author

sure - can you please let me know how exactly you applied ruff?

Also please comment on the question where the excludes are coming from.

@fkiraly, I applied Ruff by replacing the previous isort/black/flake8 pre-commit hooks with Ruff hooks and then running automated fixes. Most of the diff is formatting/lint cleanup rather than manual rewrites. For excludes/ignores, I did not copy a ready-made list from one source. I started from the project’s existing lint/format behavior, mapped that into Ruff and only added ignores when concrete migration warnings appeared.

@arnavk23
Copy link
Copy Markdown
Contributor Author

Also I think with the recent pushes, I am not able to resolve conflicts from my side so this might be hard to push afterall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Continuous integration, unit testing & package distribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MNT] move pre-commit config to ruff

2 participants