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

Apply fixes for linting issues #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peytondmurray
Copy link

@peytondmurray peytondmurray commented Oct 2, 2024

This PR applies pre-commit fixes across the entire repository in preparation for merging #40. Although this touches many files, the fixes are all safe, and were generated with ruff --fix. Most of these are end-of-file and trailing whitespace fixes, but there are also a few places imports were reordered, and where f-strings were used in place of calls to format().

Copy link
Collaborator

@hassler-google hassler-google left a comment

Choose a reason for hiding this comment

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

Hi Peyton, thanks for your contribution here.

For this PR and #38, the main thing we must figure out is how to handle the difference in the linter and documentation format between this repository and the place from which we publish changes here.

For example, the upstream repository uses a variant of pylint for linting instead of ruff, and we will need to maintain a version with pylint: https://google.github.io/styleguide/pyguide.html.

For this PR, do most of the changes go away if we set a linter check using pylint instead of ruff --fix ?

@peytondmurray
Copy link
Author

peytondmurray commented Oct 24, 2024

ruff has many rules available to enable, and pylint's rules have overlap with a lot of ruff rules.

ruff has near to complete compatibility with the rest of Python's lint tool ecosystem, except it is far, far faster, and configuration lives in one place - which makes pre-commit hooks much more pleasant to use. If at all possible I'd encourage a move to ruff; many projects in the python scientific ecosystem have done so, and it makes linting a breeze.

That said, many of the changes that you see here come from the end-of-file-fixer and trailing-whitespace pre-commit hooks, which ensure that every file ends with a newline and that useless whitespace is removed (blank spaces at the end of lines, and spaces on blank lines). Those changes have nothing to do with ruff rules, but they're good practice anyway.

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.

2 participants