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

Proposed updates to Ruff action #95

Merged
merged 1 commit into from
Nov 13, 2023
Merged

Proposed updates to Ruff action #95

merged 1 commit into from
Nov 13, 2023

Conversation

jhkennedy
Copy link
Contributor

@jhkennedy jhkennedy commented Nov 10, 2023

  • By using extend-select, we don't need to run ruff check twice

  • Adds info to README on how to go about fixing linting/formatting errors

  • Adds checks for:

    • UP: upgrade syntax for newer versions of Python
    • D: google-style docstrings on all public methods
    • ANN: use of type annotations
    • PTH: prefer pathlib over os.path

    which are all informal best practices on the team but not previously strictly enforced.

Notably, the additional checks will significantly increase the work required to align our code with this action. All work that I think is worth doing, but I'm def. open to pushback, or incrementally adding these checks.

To reduce the work required to migrate a code base, Ruff provides a way to add noqa statements to existing issues and only be strict with new issues: https://docs.astral.sh/ruff/tutorial/#adding-rules

That might be a good path forward for especially hyp3-gamma, but I'd be inclined to try and fix everything if possible and only fall back to noqa if it's too hard to switch.

See ASFHyP3/hyp3-isce2#156 for and example of what these additional checks would look like.

@jhkennedy jhkennedy requested a review from a team as a code owner November 10, 2023 06:53
Copy link
Contributor

@jtherrmann jtherrmann left a comment

Choose a reason for hiding this comment

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

I like it!

@forrestfwilliams forrestfwilliams merged commit e738f0e into ruff Nov 13, 2023
1 check passed
@forrestfwilliams forrestfwilliams deleted the ruff-ruff branch November 13, 2023 13:53
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.

3 participants