search

scikit-adaptation / skada

Domain adaptation toolbox compatible with scikit-learn and pytorch
https://scikit-adaptation.github.io/
BSD 3-Clause "New" or "Revised" License
60 stars 16 forks source link

Precommit with ruff+codespell #130

Closed agramfort closed 7 months ago

agramfort commented 7 months ago

setting up precommit hooks with ruff + codespell

codespell found a few typos :)

feedback welcome

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 99.25000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 97.62%. Comparing base (0deed68) to head (bb56347).

Files Patch % Lines
skada/datasets/_base.py 80.00% 2 Missing :warning:
skada/datasets/_samples_generator.py 96.42% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #130 +/- ## ========================================== - Coverage 97.63% 97.62% -0.01% ========================================== Files 47 47 Lines 3925 3918 -7 ========================================== - Hits 3832 3825 -7 Misses 93 93 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rflamary commented 7 months ago

@agramfort thanks for the PR, tests are not running anymore since last commit. https://github.com/scikit-adaptation/skada/actions/runs/8133112804

Could we have a quick test checking that style is valid (flake8 was not needed anymore but can we test it with ruff maybe?). To detect new contributors that did not install the pre-commit.

agramfort commented 7 months ago

@rflamary @kachayev @tgnassou this one is good to go from my end

kachayev commented 7 months ago

@agramfort This is cool, very needed tools! I have a question regarding testing/linting flow: testing job is setup to run pre-commit hooks, which contains ruff linter with --fix flag. What happens if someone commits unlinted file? I guess it will just fix the code locally in the test container without reporting/failing? 🤔

agramfort commented 7 months ago

@kachayev good question. I copied the practice of another repo that has been using this successfully for a few months. Let's see how it goes here. We can always adjust.

rflamary commented 7 months ago

well now we know it does not fail since my PR definitely does not respect ruff: https://github.com/scikit-adaptation/skada/pull/135

kachayev commented 7 months ago

That's interesting. In my PRs it actually fails because of files being changed (which is desirable) 🤔 

ruff lint skada..........................................................Failed
- hook id: ruff
- files were modified by this hook
rflamary commented 7 months ago

this is very weird no?