cookiecutter-scverse icon indicating copy to clipboard operation
cookiecutter-scverse copied to clipboard

consider adding more checks

Open flying-sheep opened this issue 3 years ago • 3 comments

After switching to Ruff in #147, we basically get a bunch of checks for free (runtime-wise and configuration-wise)

We should consider adding more!

flying-sheep avatar Feb 22 '23 10:02 flying-sheep

After all, it's very easy to customize the checks after an instance of the repo was created.

One complaint about the package from a user new to python packaging: it was difficult to remove checks if it's your first time doing a package https://github.com/scverse/scverse-tutorials/issues/35. On the other hand, it's very easy to add checks if you want them.

So, I think being conservative on checks is better given the goal and audience.

We can still add things, but I think they should be proposed individually and with a case for why they are important.

My criteria for additional checks would be:

  • Catches bugs (e.g. mutable default values, return before end of function, bare except)
  • Makes workflow significantly better (e.g. black for diffs)

Also discussed here: https://github.com/scverse/cookiecutter-scverse/issues/142#issuecomment-1439945100

ivirshup avatar Feb 22 '23 13:02 ivirshup

Good points! Ruff contains a bunch more checks, many of them much faster than their origin linters (Pylint is famously slow).

So I’m quite sure we’ll find checks matching these criteria if we go over the available rules after #147 is merged.

E.g. asyncio-dangling-task (RUF006) only comes into play if you use asyncio, but is extremely valuable as it catches a very subtle bug.

flying-sheep avatar Feb 22 '23 16:02 flying-sheep

More candidates from Zulip repo-management > Ruff

Bug catchers:

  • PD, NPY pandas, numpy best practices and gotcha avoidance

Workflow improvement:

  • TCH, FA make sure typing-only imports are in if TYPE_CHECKING blocks (making it easier to make optional dependencies optional)

flying-sheep avatar Aug 24 '23 12:08 flying-sheep