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

Replace flake8 with ruff; convert setup.cfg to pyproject.toml #3488

Closed
wants to merge 1 commit into from

Conversation

cmarqu
Copy link
Contributor

@cmarqu cmarqu commented Nov 6, 2023

No description provided.

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d47ee4) 66.87% compared to head (3579dd8) 66.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3488      +/-   ##
==========================================
- Coverage   66.87%   66.75%   -0.13%     
==========================================
  Files          49       49              
  Lines        8469     8374      -95     
  Branches     2384     2239     -145     
==========================================
- Hits         5664     5590      -74     
+ Misses       1693     1670      -23     
- Partials     1112     1114       +2     
Files Coverage Δ
src/cocotb/handle.py 84.97% <100.00%> (-0.99%) ⬇️

... and 20 files with indirect coverage changes

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

Copy link
Member

@imphil imphil left a comment

Choose a reason for hiding this comment

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

Thanks @cmarqu!

Have you tried replacing black and isort with ruff as well? (We only get the speed benefits really if we get rid of most of the tools, otherwise we're back to parsing the AST multiple times.)

If ruff works out formatting-wise, the main concern I have is that we're relying on binary components (which people cannot really build from source, a rust toolchain isn't that common these days). However, the wheels provided by the ruff project (https://pypi.org/project/ruff/#files) seem to be targeting manylinux2014/glibc 2.17, which means RHEL7 and include wheels even for s390x and ppc64lc. We seem to be loosing support for Python 3.6 and Ubuntu 18.04, which are really EOL on development machines these days.

So I think I convinced myself that these binaries are good enough. I hope the ruff project keeps up the commitment to older versions of various tools -- and if not, we can always go back to our existing setup.

Comment on lines +44 to +52
- repo: "https://github.com/tox-dev/pyproject-fmt"
rev: "1.4.1"
hooks:
- id: "pyproject-fmt"

- repo: "https://github.com/abravalheri/validate-pyproject"
rev: "v0.15"
hooks:
- id: "validate-pyproject"
Copy link
Member

Choose a reason for hiding this comment

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

These two (together with the resulting formatting changes) can go in a separate PR, or at least in a separate commit, to get them out of the way.

Maybe create a PR with another commit updating the other pre-commit tool versions.

@ktbarrett
Copy link
Member

I wonder if we could get the ruff guys to do fix psf/black#2156 since the black authors are so unwilling to change that awful formatting strategy.

@cmarqu
Copy link
Contributor Author

cmarqu commented Nov 9, 2023

Superseded.

@cmarqu cmarqu closed this Nov 9, 2023
@ktbarrett ktbarrett deleted the chore/use-ruff branch December 1, 2023 03:38
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.

None yet

3 participants