Skip to content
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

use ruff for code linting #490

Merged
merged 6 commits into from
Aug 12, 2024
Merged

use ruff for code linting #490

merged 6 commits into from
Aug 12, 2024

Conversation

mathause
Copy link
Member

@mathause mathause commented Aug 8, 2024

Add ruff configuration. I don't necessarily want to switch but we could...

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

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

Project coverage is 52.77%. Comparing base (9b0b76b) to head (67f85a7).
Report is 126 commits behind head on main.

Files with missing lines Patch % Lines
mesmer/mesmer_x/OLD_train_l_distrib.py 0.00% 2 Missing ⚠️
mesmer/mesmer_x/temporary_config_all.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #490       +/-   ##
===========================================
- Coverage   87.90%   52.77%   -35.14%     
===========================================
  Files          40       49        +9     
  Lines        1745     3335     +1590     
===========================================
+ Hits         1534     1760      +226     
- Misses        211     1575     +1364     
Flag Coverage Δ
unittests 52.77% <25.00%> (-35.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@veni-vidi-vici-dormivi
Copy link
Collaborator

Would it replace sort and flake8? The changes it made are nice. In #343 you said we would need to update our documentation, why? Also that we should wait for an update so that it does not interfere with black, is that out now?

@mathause
Copy link
Member Author

mathause commented Aug 8, 2024

Right - I made an issue on that 😳 the formatting tools are mentioned in https://github.com/MESMER-group/mesmer/blob/main/docs/source/development.rst#formatting so they'd need updating (and also the Makefile, but we could also delete it), and it's less important now that we use pre-commit CI anyways.

The formatting issue is still open - astral-sh/ruff#9745

@mathause
Copy link
Member Author

mathause commented Aug 8, 2024

And yes it would replace flake8 and isort (and we could consider adding more rules, see e.g. https://github.com/shapely/shapely/blob/775f7792ecd945fa30b3f98b46d99f996e275f00/pyproject.toml#L81 for an inspiration (or also the ruff docs).

pyproject.toml Outdated Show resolved Hide resolved
@mathause mathause changed the title add ruff configuration use ruff for code linting Aug 8, 2024
@mathause
Copy link
Member Author

mathause commented Aug 8, 2024

Ok, switched to ruff entirely. I keep the isort and flake config around - otherwise we can get a mess if isort or flake is ever called. One issue with this is that there are formatting checks in ruff - so they might trigger an error that is fixed by black. Before we could run the formatting checks after the formater. If that is a problem we can turn off the formatting checks: astral-sh/ruff-pre-commit#74 (comment)

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@veni-vidi-vici-dormivi
Copy link
Collaborator

And think about what happens to #343 and #56 now.

Copy link
Member Author

@mathause mathause left a comment

Choose a reason for hiding this comment

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

Thanks for the review. I will update the two files. We can close #343 but I leave #56 for another PR.

We should maybe remove the Makefile - I don't actually use it and haven't checked if it still works (I mean I guess it does but still).

Makefile Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@mathause mathause merged commit 845b1e1 into MESMER-group:main Aug 12, 2024
9 checks passed
@mathause mathause deleted the ruff_config branch August 12, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use ruff as linter?
2 participants