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 pyupgrade isort and black with ruff format #983

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zacharyburnett
Copy link
Collaborator

This PR replaces the pre-commit checks for pyupgrade, isort, and black with ruff format

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
romancal/resample/gwcs_drizzle.py 80.68% <ø> (ø)
romancal/skymatch/skymatch.py 80.22% <ø> (ø)
romancal/ramp_fitting/ramp_fit_step.py 31.70% <0.00%> (-0.82%) ⬇️

... and 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

@WilliamJamieson
Copy link
Contributor

In order to actually make this a replacement for all the claimed tools, the rule sets for isort, etc have to be enabled in the pyproject.toml file. Those are not part of the default rule set.

@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Nov 9, 2023
@zacharyburnett zacharyburnett marked this pull request as ready for review November 9, 2023 19:47
@zacharyburnett zacharyburnett requested a review from a team as a code owner November 9, 2023 19:47
Copy link
Collaborator

@schlafly schlafly left a comment

Choose a reason for hiding this comment

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

This looks fine to me, and I'm probably in favor of fewer moving pieces, which seems to be the goal of this PR.

But I do think we want to ~settle on some choice of code formatting, etc., and not mess with it very often. Does this represent that choice?

@zacharyburnett
Copy link
Collaborator Author

This looks fine to me, and I'm probably in favor of fewer moving pieces, which seems to be the goal of this PR.

But I do think we want to ~settle on some choice of code formatting, etc., and not mess with it very often. Does this represent that choice?

We can configure ruff format a bit more than we can Black, namely line length, quote character, indent style, etc.: https://docs.astral.sh/ruff/formatter/#configuration

'build',
]
ignore = [
'E741', # ambiguous variable name
]
extend-select = ['NPY']
extend-include = ["*.ipynb"]

[tool.ruff.lint.isort]
Copy link
Contributor

Choose a reason for hiding this comment

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

This still is not selecting the rules for ruff to replace the pre-commit tooling this PR claims to be replacing. I'll open a PR against your branch fixing these issues.

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.

None yet

3 participants