Skip to content

Conversation

@peytondmurray
Copy link
Collaborator

@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
Collaborator 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.

@smokestacklightnin
Copy link
Collaborator

smokestacklightnin commented Aug 15, 2025

@peytondmurray @andrewfulton9 can I please get your approval of my changes on this PR?

@peytondmurray
Copy link
Collaborator Author

Let's hold off on merging until we get tests running, which are currently blocked by the build being broken.

@peytondmurray peytondmurray marked this pull request as draft August 15, 2025 05:35
@andrewfulton9
Copy link

I may just be missing something, but where are the configurations? It looks like the pyproject.toml is empty and I don't see any other linting configurations here.

@peytondmurray
Copy link
Collaborator Author

The configuration will be in #40 - this PR isolates the changes that #40 would make on the formatting so that we can have confidence the (many) changes you see here don't break tests.

Until we get tests running, I think it makes sense to hold off here.

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.

4 participants