-
Notifications
You must be signed in to change notification settings - Fork 195
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
Python: Errors & Linting #5124
Python: Errors & Linting #5124
Conversation
@@ -405,7 +407,7 @@ | |||
ds_start = yt.load(filename_start) | |||
ad_end = ds_end.all_data() | |||
ad_start = ds_start.all_data() | |||
dt = float(ds_end.current_time - ds_start.current_time) | |||
dt = float(ds_end.current_time - ds_start.current_time) # noqa |
Check notice
Code scanning / CodeQL
Unused local variable Note
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.
@dpgrote wrote in #5123 (comment)
The
eval
below needs dt. Theeval
is kind of hacky anyway. Something like this would be betterif i == 1: specific_check1(data, dt) elif i == 2: specific_check2(data, dt)
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.
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.
A simple fix would be to do
eval("specific_check"+str(i))(data, dt)
But it would be better to remove the eval altogether.
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.
Yes, that looks very similar to me 😅
21c1c83
to
e9ffd2a
Compare
8a9e3b1
to
06cb199
Compare
Add no-quality-assurance (noqa) annotations.
Removes unmaintained and outdated performance tests we ran on older HPC machines. We will reestablish something more maintainable with CTest and NESAP workflows through containers.
06cb199
to
1262ee6
Compare
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.
Looks good to me.
Trying to fix a lot of errors so far in preparation of integrating Ruff in #5123.
Removes unmaintained and outdated performance tests we ran on older machines. We will reestablish something more maintainable with CTest and NESAP workflows through containers.
First badge of fixes, ready to go.