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

ci(ruff): add extra linting rules: isort, pyupgrade, numpy, ruff-specific #243

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

ljgray
Copy link
Contributor

@ljgray ljgray commented May 5, 2023

These additions are useful for improving overall quality and consistency of code and eliminating extra things for PR reviewers to check. The ruff python version is set to python 3.8, since this is the minimum supported version.

  • isort: formats and alphabetically orders imports
  • pyupgrade: upgrade syntax for newer versions of python (based on the version specified in pyproject.toml)
  • numpy: check deprecated types
  • ruff-specific: ambiguous unicode, collection instead of concatenation
  • flake8-blind-except: don't allow blind except statements
  • flake8-comprehensions: use comprehensions instead of concatenation where possible
  • flake8-return: consistent and clear return statements

Additional ignored rules:

  • NPY002: replace legacy numpy.random calls with np.random.Generator

I'm happy to add more rules if there's anything else of interest

@jrs65
Copy link
Contributor

jrs65 commented May 5, 2023

I don't think you can use the | syntax instead on Union until 3.10.

@ljgray
Copy link
Contributor Author

ljgray commented May 5, 2023

Good catch. This PR will probably look pretty messy until I nail down exactly which rules to use.

@ljgray ljgray force-pushed the extra-ruff-rules branch 2 times, most recently from ff0d3ed to 05f0ded Compare May 6, 2023 00:11
@ljgray ljgray requested review from jrs65 and ketiltrout May 6, 2023 00:32
@ljgray
Copy link
Contributor Author

ljgray commented May 11, 2023

I don't think you can use the | syntax instead on Union until 3.10.

Actually, in hindsight this is exactly what from __future__ import annotations is for, so we could potentially include that everywhere it's needed

@ketiltrout
Copy link
Member

I don't think you can use the | syntax instead on Union until 3.10.

Actually, in hindsight this is exactly what from __future__ import annotations is for, so we could potentially include that everywhere it's needed

from __future__ import annotations causes Python to treat annotations as if they were strings. So, while this import allows using | syntax when running in pre-3.10 Python, a pre-3.10 type checker will still break when encountering | instead of Union.

@ljgray ljgray force-pushed the extra-ruff-rules branch 4 times, most recently from 8e0ac2f to b0c7511 Compare May 18, 2023 18:03
@ljgray ljgray marked this pull request as ready for review May 18, 2023 18:09
@ljgray ljgray requested a review from ketiltrout June 9, 2023 18:35
@ljgray
Copy link
Contributor Author

ljgray commented Nov 2, 2023

@ketiltrout @jrs65 I've done another run through to make sure everything is working as expected here, so I want to ping you both for review again.

@ljgray ljgray merged commit 1fb52bf into master Nov 17, 2023
@ljgray ljgray deleted the extra-ruff-rules branch November 17, 2023 23:25
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.

3 participants