-
Notifications
You must be signed in to change notification settings - Fork 7
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
add ruff formatter #297
add ruff formatter #297
Conversation
WalkthroughThe recent updates enhance string manipulation and dependency handling in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 7
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- .ci_support/release.py (3 hunks)
- .pre-commit-config.yaml (1 hunks)
- pysqa/init.py (1 hunks)
- pysqa/executor/init.py (1 hunks)
- pysqa/wrapper/lsf.py (1 hunks)
- setup.py (1 hunks)
- tests/test_basic.py (1 hunks)
- tests/test_cmd.py (5 hunks)
- tests/test_execute_command.py (1 hunks)
- tests/test_executor.py (6 hunks)
- tests/test_flux.py (5 hunks)
- tests/test_gent.py (4 hunks)
- tests/test_lsf.py (3 hunks)
- tests/test_moab.py (1 hunks)
- tests/test_multi.py (2 hunks)
- tests/test_remote.py (4 hunks)
- tests/test_scheduler_commands.py (1 hunks)
- tests/test_sge.py (3 hunks)
- tests/test_slurm.py (7 hunks)
- tests/test_torque.py (2 hunks)
Files skipped from review due to trivial changes (6)
- .pre-commit-config.yaml
- setup.py
- tests/test_cmd.py
- tests/test_execute_command.py
- tests/test_lsf.py
- tests/test_torque.py
Additional Context Used
Ruff (11)
.ci_support/release.py (4)
3-3: Ambiguous variable name:
l
25-25: Ambiguous variable name:
l
46-46: Ambiguous variable name:
l
49-49: Ambiguous variable name:
l
tests/test_flux.py (2)
5-5:
time.sleep
imported but unused
13-13:
flux
imported but unused; consider usingimportlib.util.find_spec
to test for availabilitytests/test_multi.py (2)
9-9:
paramiko
imported but unused; consider usingimportlib.util.find_spec
to test for availability
10-10:
tqdm.tqdm
imported but unused; consider usingimportlib.util.find_spec
to test for availabilitytests/test_remote.py (2)
9-9:
paramiko
imported but unused; consider usingimportlib.util.find_spec
to test for availability
10-10:
tqdm.tqdm
imported but unused; consider usingimportlib.util.find_spec
to test for availabilitytests/test_sge.py (1)
11-11:
defusedxml.ElementTree
imported but unused; consider usingimportlib.util.find_spec
to test for availability
Additional comments not posted (48)
pysqa/executor/__init__.py (1)
2-3
: LGTM! The import and__all__
declaration are correctly added.pysqa/__init__.py (1)
5-5
: LGTM! The import and__all__
declaration are correctly added.tests/test_multi.py (2)
26-36
: LGTM! TheskipIf
decorator andsetUpClass
method are correctly implemented.
43-43
: LGTM! The test methods are correctly implemented.tests/test_moab.py (3)
Line range hint
1-6
: LGTM! The imports are correctly implemented.
Line range hint
15-15
: LGTM! The class definition andsetUpClass
method are correctly implemented.
28-28
: LGTM! The test methods are correctly implemented.tests/test_scheduler_commands.py (3)
Line range hint
1-2
: LGTM! The imports are correctly implemented.
Line range hint
5-7
: LGTM! The class definitions and methods are correctly implemented.
Line range hint
20-20
: LGTM! The test methods are correctly implemented.pysqa/wrapper/lsf.py (1)
43-44
: Improved variable naming for readability.tests/test_basic.py (1)
67-67
: Improved formatting for readability.tests/test_flux.py (1)
34-34
: Updated string literals to use double quotes for consistency.Also applies to: 46-48, 57-71, 75-95, 118-122
tests/test_remote.py (1)
26-29
: Updated string literals to use double quotes for consistency.Also applies to: 35-37, 44-44, 54-56, 74-74, 79-82, 85-88, 93-93, 99-99, 107-111
tests/test_executor.py (4)
23-23
: Good formatting improvement for readability.
26-26
: Appropriate use ofskipIf
to handle platform-specific issues.
29-31
: Good formatting improvement for readability.
42-42
: Good formatting improvement for readability.tests/test_sge.py (2)
26-29
: Appropriate use ofskipIf
to handle missing dependencies.
41-41
: Good standardization of string representation to use double quotes.tests/test_gent.py (22)
22-28
: Good formatting improvement for readability.
33-33
: Good use of a blank line to separate code blocks for better readability.
45-45
: Good standardization of string representation to use double quotes.
71-73
: Good standardization of string representation to use double quotes.
76-78
: Good standardization of string representation to use double quotes.
94-94
: Good standardization of string representation to use double quotes.
99-102
: Good standardization of string representation to use double quotes.
107-111
: Good formatting improvement for readability.
117-118
: Good formatting improvement for readability.
119-126
: Good formatting improvement for readability.
131-135
: Good formatting improvement for readability.
141-142
: Good formatting improvement for readability.
143-151
: Good formatting improvement for readability.
156-160
: Good formatting improvement for readability.
166-166
: Good formatting improvement for readability.
172-176
: Good formatting improvement for readability.
182-182
: Good formatting improvement for readability.
188-193
: Good formatting improvement for readability.
199-199
: Good formatting improvement for readability.
201-204
: Good formatting improvement for readability.
209-213
: Good formatting improvement for readability.
220-225
: Good formatting improvement for readability.tests/test_slurm.py (6)
23-29
: Good formatting improvement for readability.
37-37
: Good formatting improvement for readability.
53-53
: Good standardization of string representation to use double quotes.
118-118
: Good formatting improvement for readability.
127-127
: Good formatting improvement for readability.
153-154
: Good formatting improvement for readability.
@@ -9,6 +9,7 @@ | |||
|
|||
try: | |||
import defusedxml.ElementTree as ETree | |||
|
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.
Remove the unused import defusedxml.ElementTree
.
- import defusedxml.ElementTree as ETree
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import paramiko | ||
from tqdm import tqdm |
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.
Remove unused imports paramiko
and tqdm
.
- import paramiko
- from tqdm import tqdm
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import paramiko | |
from tqdm import tqdm |
@@ -1,12 +1,19 @@ | |||
def get_setup_version_and_pattern(setup_content): | |||
depend_lst, version_lst = [], [] | |||
for l in setup_content: |
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.
Rename the variable l
to a more descriptive name for clarity.
- for l in setup_content:
+ for line in setup_content:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
for l in setup_content: | |
for line in setup_content: |
@@ -16,14 +23,14 @@ | |||
read_flag = False | |||
depend_lst, version_lst = [], [] | |||
for l in env_content: |
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.
Rename the variable l
to a more descriptive name for clarity.
- for l in env_content:
+ for line in env_content:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
for l in env_content: | |
for line in env_content: |
@@ -35,27 +42,29 @@ | |||
version_combo_dict[dep] = dep + "==" + ver | |||
|
|||
setup_content_new = "" | |||
pattern_dict = {d:d + "==" + v for d, v in version_high_dict.items()} | |||
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()} | |||
for l in setup_content: |
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.
Rename the variable l
to a more descriptive name for clarity.
- for l in setup_content:
+ for line in setup_content:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
for l in setup_content: | |
for line in setup_content: |
@@ -11,12 +11,15 @@ | |||
|
|||
try: | |||
import flux |
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.
Consider using importlib.util.find_spec
to check for module availability instead of importing flux
.
- import flux
+ import importlib.util
+ flux_spec = importlib.util.find_spec("flux")
+ skip_flux_test = flux_spec is None
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import flux | |
import importlib.util | |
flux_spec = importlib.util.find_spec("flux") | |
skip_flux_test = flux_spec is None |
import paramiko | ||
from tqdm import tqdm |
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.
Consider using importlib.util.find_spec
to check for module availability instead of importing paramiko
and tqdm
.
- import paramiko
- from tqdm import tqdm
+ import importlib.util
+ paramiko_spec = importlib.util.find_spec("paramiko")
+ tqdm_spec = importlib.util.find_spec("tqdm")
+ skip_remote_test = paramiko_spec is None or tqdm_spec is None
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
import paramiko | |
from tqdm import tqdm | |
import importlib.util | |
paramiko_spec = importlib.util.find_spec("paramiko") | |
tqdm_spec = importlib.util.find_spec("tqdm") | |
skip_remote_test = paramiko_spec is None or tqdm_spec is None |
Summary by CodeRabbit
New Features
QueueAdapter
to the exported list for easier access.Bug Fixes
Documentation
ruff lint
andruff format
to maintain code quality.Refactor
Style
Tests