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 linter pre-commit actions #824

Merged
merged 49 commits into from
Jan 4, 2023
Merged

use Ruff linter pre-commit actions #824

merged 49 commits into from
Jan 4, 2023

Conversation

marscher
Copy link
Contributor

@marscher marscher commented Nov 15, 2022

Changes

  • Use Ruff linter pre-commit actions
  • Fixed linting issues.
  • Use Ruff with NbQA.
  • Removed (replaced with Ruff) pre-commit hooks:
    • isort
    • pydocstyle
    • flake8
    • pep585-upgrade

Note that I also removed flake8-deprecations, which only checked for a hard coded dictionary of super old deprecations inside the std lib.

Checks

  • updated CHANGELOG.rst

@github-actions
Copy link

github-actions bot commented Nov 15, 2022

Test Results

2 184 tests  ±0   2 183 ✔️ ±0   3m 37s ⏱️ +58s
       1 suites ±0          1 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit 84ec757. ± Comparison against base commit db750a4.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #824 (84ec757) into master (db750a4) will increase coverage by 0.00%.
The diff coverage is 98.48%.

@@           Coverage Diff           @@
##           master     #824   +/-   ##
=======================================
  Coverage   96.82%   96.83%           
=======================================
  Files          92       92           
  Lines        6055     6071   +16     
=======================================
+ Hits         5863     5879   +16     
  Misses        192      192           
Impacted Files Coverage Δ
weldx/geometry.py 96.61% <ø> (ø)
weldx/transformations/local_cs.py 96.34% <ø> (ø)
weldx/welding/groove/iso_9692_1.py 99.52% <ø> (ø)
weldx/transformations/cs_manager.py 98.60% <80.00%> (+<0.01%) ⬆️
weldx/asdf/file.py 96.27% <100.00%> (+0.06%) ⬆️
weldx/asdf/util.py 91.19% <100.00%> (ø)
weldx/constants.py 100.00% <100.00%> (ø)
weldx/core/generic_series.py 89.29% <100.00%> (+0.09%) ⬆️
weldx/core/time_series.py 97.72% <100.00%> (+0.01%) ⬆️
weldx/measurement.py 95.62% <100.00%> (+0.01%) ⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CagtayFabry
Copy link
Member

I think the config looks very clean now that nbqa-ruff is also available @marscher 👍

Just some errors to look into with the latest versions

@CagtayFabry
Copy link
Member

we could also think about removing isort for the ruff implementation

@marscher marscher marked this pull request as ready for review January 2, 2023 07:34
@marscher
Copy link
Contributor Author

marscher commented Jan 2, 2023

With ruff-0.0.208 the error output does not contain the originating file name. This renders the output useless.

@CagtayFabry
Copy link
Member

With ruff-0.0.208 the error output does not contain the originating file name. This renders the output useless.

oh that sounds bad, I hope ruff will introduce an option to toggle this at least..?

Copy link
Member

@CagtayFabry CagtayFabry left a comment

Choose a reason for hiding this comment

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

wow great work digging through all of it @marscher 🙏
the linting config also looks super clean 👍

@marscher
Copy link
Contributor Author

marscher commented Jan 4, 2023

Thanks!

@marscher marscher merged commit b16cec4 into BAMWelDX:master Jan 4, 2023
@marscher marscher deleted the ruff branch January 4, 2023 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants