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

Use try/except to catch error #917

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions src/aiidalab_qe/setup/codes.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
import subprocess
from pathlib import Path
from shutil import which
Expand Down Expand Up @@ -44,18 +45,45 @@ def get_qe_env():


def qe_installed():
import json
"""Check if Quantum Espresso (QE) is installed in the specified conda environment.

env_exist = get_qe_env().exists()
proc = subprocess.run(
["conda", "list", "-n", f"{get_qe_env().name}", "--json", "--full-name", "qe"],
check=True,
capture_output=True,
)
Returns:
bool: True if the environment exists and QE is installed; False otherwise.
"""
try:
# Verify if the specified conda environment exists
env_exist = get_qe_env().exists()

if not env_exist:
return False
Comment on lines +55 to +58
Copy link
Member Author

Choose a reason for hiding this comment

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

Because we check if the conda env exists here, I didn't add the code to run the code cmd using the subprocess.


# Run the conda list command to check for the QE package
proc = subprocess.run(
[
"conda",
"list",
"-n",
f"{get_qe_env().name}",
"--json",
"--full-name",
"qe",
],
check=True,
capture_output=True,
)

info = json.loads(str(proc.stdout.decode()))[0]
# Load and interpret the JSON output
info = json.loads(proc.stdout.decode())

return env_exist and "qe" == info["name"]
# Check if 'qe' is listed in the environment
for package in info:
if package.get("name") == "qe":
return True
return False # noqa: TRY300
except Exception as error:
raise RuntimeError(
"Failed to check if Quantum Espresso is installed."
) from error
Comment on lines +82 to +86
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return False # noqa: TRY300
except Exception as error:
raise RuntimeError(
"Failed to check if Quantum Espresso is installed."
) from error
except Exception as exc:
raise RuntimeError(
"Failed to check if Quantum Espresso is installed."
) from exc
else:
return False # noqa: TRY300

Also for the catched exception, I think there are two types of exceptions we know which are SubprocessCallError and JsonParseError. You can catch those two and for uncatched exceptions, use raise to propagated it up.

Copy link
Member

Choose a reason for hiding this comment

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

For TRY300, you can move it to else block, https://docs.astral.sh/ruff/rules/try-consider-else/



def install_qe():
Expand Down
Loading