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

consider adding more checks #149

Open
flying-sheep opened this issue Feb 22, 2023 · 3 comments
Open

consider adding more checks #149

flying-sheep opened this issue Feb 22, 2023 · 3 comments

Comments

@flying-sheep
Copy link
Member

flying-sheep commented Feb 22, 2023

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!

@ivirshup
Copy link
Member

ivirshup commented Feb 22, 2023

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 scverse/scverse-tutorials#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: #142 (comment)

@flying-sheep
Copy link
Member Author

flying-sheep commented Feb 22, 2023

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
Copy link
Member Author

flying-sheep commented Aug 24, 2023

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)

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

No branches or pull requests

2 participants