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

Python: Errors & Linting #5124

Merged
merged 9 commits into from
Aug 18, 2024
Merged

Python: Errors & Linting #5124

merged 9 commits into from
Aug 18, 2024

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Aug 10, 2024

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.

@ax3l ax3l added cleaning Clean code, improve readability component: Python Python layer labels Aug 10, 2024
Docs/source/conf.py Dismissed Show resolved Hide resolved
@@ -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

Variable dt is not used.
Copy link
Member Author

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. The eval is kind of hacky anyway. Something like this would be better

if i == 1:
    specific_check1(data, dt)
elif i == 2:
    specific_check2(data, dt)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NeilZaim @RemiLehe can you quickly see how to remove the eval with a more explicit or handling in this function?

Copy link
Member

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.

Copy link
Member Author

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 😅

@ax3l ax3l mentioned this pull request Aug 10, 2024
2 tasks
@ax3l ax3l requested a review from NeilZaim August 10, 2024 19:21
@ax3l ax3l force-pushed the fix-python-linter branch 2 times, most recently from 21c1c83 to e9ffd2a Compare August 12, 2024 22:19
@ax3l ax3l force-pushed the fix-python-linter branch 2 times, most recently from 8a9e3b1 to 06cb199 Compare August 14, 2024 22:43
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.
@ax3l
Copy link
Member Author

ax3l commented Aug 15, 2024

@RemiLehe @dpgrote I think we could merge this already, there will be more in #5123

Copy link
Member

@roelof-groenewald roelof-groenewald left a 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.

@ax3l ax3l merged commit 127b5f4 into ECP-WarpX:development Aug 18, 2024
47 checks passed
@ax3l ax3l deleted the fix-python-linter branch August 18, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Clean code, improve readability component: Python Python layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants