pyvo icon indicating copy to clipboard operation
pyvo copied to clipboard

Move to pyproject.toml from setup.{py/cfg} and add a minimal ruff linter/formatter

Open MridulS opened this issue 1 year ago • 6 comments

I started working on https://github.com/astropy/pyvo/issues/155 and I thought it would be helpful to clean up this repo a bit before.

I have moved around a bunch of stuff in this PR, let me know if it makes easier to review if I break this up in smaller pull requests :)

  • moved all the config from setup.cfg to pyproject.toml
  • Removed setup.py, moved the version handling to setuptools_scm through pyproject.toml
  • Add a ruff linter and formatter to pre-commit to bring the linting in this repo up to date with astropy/astropy

I hope I didn't break anything here 😅

python -m build was able to build a wheel successfully and I was able to run pytest with no failures.

I have not run the ruff linter yet! Once we have a positive review I can run the pre-commit bits to format the code (didn't want to pollute the diff too much).

MridulS avatar Feb 20 '24 06:02 MridulS

Another thing is that I have removed the coverage, flake8, pycodestyle specific configuration. I can add them back but I am not a 100% it makes sense to keep this along the pre-commit ruff action :)

MridulS avatar Feb 20 '24 06:02 MridulS

Add a ruff linter and formatter to pre-commit to bring the linting in this repo up to date with astropy/astropy

Please no, we are talking about turning off more formatting stuff than we already have and this goes the opposite direction. But I let @msdemlei to chime in, too.

bsipocz avatar Feb 20 '24 20:02 bsipocz

Please no, we are talking about turning off more formatting stuff than we already have and this goes the opposite direction. But I let @msdemlei to chime in, too.

No worries, we don't have to add more linters :)

Is consolidating everything in pyproject.toml okay? I think it really helps to have everything in one place as a single source of truth.

MridulS avatar Feb 21 '24 03:02 MridulS

On Tue, Feb 20, 2024 at 12:47:24PM -0800, Brigitta Sipőcz wrote:

Add a ruff linter and formatter to pre-commit to bring the linting in this repo up to date with astropy/astropy

Please no, we are talking about turning off more formatting stuff than we already have and this goes the opposite direction. But I let @msdemlei to chime in, too.

Well... I freely admit that my favourite passage in PEP 8 is

However, know when to be inconsistent – sometimes style guide recommendations just aren’t applicable. When in doubt, use your best judgment. Look at other examples and decide what looks best.

And indeed, in my code that others don't generally contribute to, I'm quite a distance remote from PEP 8 (starting with my uncurable affection for tabs).

On the other hand, I totally see that we want to minimise the number of diff lines only dealing with whitespace and formatting, and so I certainly won't veto committing to as many conventions as are necessary to achieve that goal. Going beyond that, I'm more skeptical. And I am particularly skeptical about having a machine re-format source code. But maybe that's just the result of having been bitten by inferior tools of yesteryear.

So: If all it takes for ruff-ing things up is a few dozen lines of diff, I won't say no. If the result is thousands of lines of diff, I'd pull a Bartleby ("I would prefer not to").

msdemlei avatar Feb 21 '24 08:02 msdemlei