-
Notifications
You must be signed in to change notification settings - Fork 2
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
MAINT: Test using ruff with minimal changes to defaults #127
Conversation
Note that this uses much fewer rules than scipp/scipp#3424 In particular, it does not use rules based on isort, bugbear, and bandit. So we still need to run those linters. |
urghh, for some reason I thought the default was running all the rules (it's not). Need to select ALL. |
"PT", "PYI", "RUF", "UP", "W"] | ||
ignore = [ | ||
"E111", "E114", "E117", "D206", "D300", # conflict with ruff format | ||
"UP007"] # FIXME: PEP 604 breaks esssans test suite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright the problem is that this rule only works for python 3.10+ which essans
satisfies but in the test suite it uses a released version of sciline which is on py3.9.
So I end up with errors like E TypeError: 'type' object is not subscriptable
locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would this matter? We don't lint our dependencies, are we?
@@ -248,7 +247,7 @@ | |||
")\n", | |||
"parts = (CleanSummedQ[SampleRun, Numerator], CleanSummedQ[SampleRun, Denominator])\n", | |||
"iofqs = (IofQ[SampleRun], IofQ[BackgroundRun], BackgroundSubtractedIofQ)\n", | |||
"keys = monitors + (MaskedData[SampleRun],) + parts + iofqs\n", | |||
"keys = (*monitors, MaskedData[SampleRun], *parts, *iofqs)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this preferred...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more efficient. Plus it avoids the awkward (x,)
syntax.
@@ -20,14 +19,11 @@ | |||
""" | |||
|
|||
|
|||
def write_dependencies(dependency_name: str, dependencies: List[str]) -> None: | |||
def write_dependencies(dependency_name: str, dependencies: list[str]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this preferred...? Or is it only for consistent type-hinting...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using generics is deprecated behaviour https://docs.astral.sh/ruff/rules/non-pep585-annotation/
I'll close this one, once scipp/scipp#3424 is merged in we should create one in the template and let the github action do the work. |
This adds an example ruff config which uses all the defaults with minimal config. It makes very little changes.