Use Ruff linter and formatter

[ddundo]

Closes #54.

This PR introduces the Ruff linter and formatter to check for errors and stylistic issues, including the pre-commit hook. Relevant changes are done in pyproject.toml and test_suite.yml. The rest of changes are due to applying the fixes.

A few things to note:

• I updated contribution guidelines
• I added the ruff linting and formatting checks to the test_suite.yml workflow
• The maximum line length is set to 88 by default, which we can change if you'd prefer
• At the moment I ignored quite a few errors that couldn't be fixed automatically (see the ignore list in pyproject.toml) - I'll open a separate issue to address these. Edit: opened Fix the rest of linting rules #68
• I added sample_data/ to .gitignore

[ddundo]

Hi @btobers @drounce, I updated this with the latest changes on dev. Lots of changes, but only about 40 lines of code are actual changes that I have made manually:

• pyproject.toml, where ruff is configured
• test_suite.yml, where I added the linting and formatting check to be run automatically (see the last few CI runs to see what it looks like)
• Commit b0637f7 where I update contribution guidelines with instructions

Then I applied the automatic fixes:

• Linting in commit 722e9de - this took care of unused and unsorted imports, trailing whitespaces...
• Formatting in commit 9cb48bf
  • A lot of changes are due to changing single quotation marks to double (default). In the original code both single and double are used, so I was not sure what your preference is. We can change this.
  • Limiting line length to 88 characters (default) where possible. We can change this too.
  • Adding whitespaces around arithmetic operators
  • etc.

Ruff follows the Black code style (see https://black.readthedocs.io/en/stable/the_black_code_style/index.html) which seems to be the most popularly used one lately. Please take a look there and see if there is something which you don't like. I don't think there's anything controversial.

[btobers]

Hi @ddundo thanks for working on this! I have no issues with the Black code style.

Question: you added ruff linter instructions to the contribution guide in commit b0637f7. However, you also set up a ruff check in test_suite.yml. Would this make is so that the GitHub tests workflow would fail if ruff linter encounters an unresolved issue? In other words, we need to run ruff check --fix or ruff format and commit any changes when submitting a PR? If this is the case, I think this should clearly be stated in the contribution guide to note that our GitHub tests workflow may fail if ruff linter is not run. Is there a way (or would it make sense) to set up a GitHub workflow to run ruff linter and fix any encountered formatting issues automatically. I imagine this might be challenging because of the need to commit any codespace changes.

[ddundo]

Would this make is so that the GitHub tests workflow would fail if ruff linter encounters an unresolved issue? In other words, we need to run ruff check --fix or ruff format and commit any changes when submitting a PR?

Yup, that's right.

If this is the case, I think this should clearly be stated in the contribution guide to note that our GitHub tests workflow may fail if ruff linter is not run.

Good point, added in 75fc63e.

Is there a way (or would it make sense) to set up a GitHub workflow to run ruff linter and fix any encountered formatting issues automatically. I imagine this might be challenging because of the need to commit any codespace changes.

This is possible, but if we wanted this to be run automatically I'd prefer to add a pre-commit hook. I actually added this but then dropped it (see 0210aa1 on how it looks). What this does is automatically run the linter and formatter when we try to commit. The commit will fail if issues are detected. I personally like this approach, but some dislike it because they feel like it interferes with their work too much. And since we have these checks being run as part of the test suite, these issues will anyway need to be resolved before merging. But this leaves it to the developer to decide at what point to address it, rather than being forced to address it every time they try to commit.

[btobers]

That makes sense, and I can see the argument for not automatically fixing issues with a pre-commit hook. Thanks!

[btobers]

I looked everything over and don't see any issues. As you mentioned @ddundo nearly all the codespace changes are formatting from ruff linter. However, I'll let @drounce look this over and approve just to make sure he's okay with everything also.

[drounce]

My preference would be for single quotes. I don't see the ability to review this, so will circle back, but based on your stated summary this seems like a good path forward.

[ddundo]

Thanks both! I changed to single quotes in d043d47, which involved setting

[tool.ruff.format]
quote-style = 'single'

in pyproject.toml.

And sorry, I just realised I didn't request reviews earlier.

[btobers]

@ddundo, I'm not sure my previous review went through successfully. This looks great. I just left a couple suggestions to further clarify that ruff linting and formatting must both be completed prior to merging a PR.

[ddundo]

Thanks @btobers! I committed your suggestions and will go ahead and merge.