-
Notifications
You must be signed in to change notification settings - Fork 79
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
qcengine info psi4 harness raises exception if system uses pyenv #292
Comments
Thanks for the detailed analysis. Is https://github.com/MolSSI/QCEngine/blob/master/qcengine/programs/psi4.py#L55 coming back |
Yes. Because if you use
|
I've a reported similar issue: https://gist.github.com/sassy-crick/f5b058bc9c4fd7be44d730eb17ec4e6d It's from the test suite of PSI4 and the relevant log part is:
I'm wondering why the code is seemingly overly complicated: https://github.com/MolSSI/QCEngine/blob/v0.24.1/qcengine/programs/psi4.py#L67-L72
So I'd strongly suggest to add any kind of error checking first before accessing Additionally fully replacing So why not simply import psi4 and directly access EDIT: Overwriting |
When `psi4` binary is found but the Python package is not we should not overwrite the `$PYTHONPATH` before importing `psi4` as that may cause other required packages to be not found anymore. Hence prepend it. See MolSSI#292
I think #418 solves @Flamefire 's issues but not @coltonbh 's original one from #292 (comment) . I've read over pyenv's documentation, and it sounds like pyenv is a |
Here's a function that does a quick fix for the def prog_available(program: str) -> bool:
"""Check if a program is available on the system.
Args:
program: The program to check.
Returns:
True if the program is available, False otherwise.
"""
path = shutil.which(program)
if not path:
return False
# Perform secondary checks
if ".pyenv/shims" in path:
# Executable is in non activated pyenv environment; often a conda env
print(
f"Program {program} is in a pyenv shim. It may become available if you "
f"run 'conda activate {path.split('/')[-1]}'"
)
return False
return True |
I think #418 already does solve this by https://github.com/MolSSI/QCEngine/pull/418/files#diff-d66842b418a1c03f41438563803c987c9defd1811ea0af131e285ba540c8c0d6R70 According to the initial description However it might be that https://github.com/MolSSI/QCEngine/pull/418/files#diff-d66842b418a1c03f41438563803c987c9defd1811ea0af131e285ba540c8c0d6R69 is wrong treating an empty So checking the return code and/or stdout/stderr should also already solve the pyenv/condaenv issues, doesn't it? |
MolSSI/QCElemental#322 should address the original issue now, regardless of stderr empty or not. The case with empty stdout turning into I'll plan on closing this issue with 418. Please let me know if you spy any more real or potential problems. |
Describe the bug
Running
qcengine info
raises the following error if a system has installed python versions usingpyenv
.To Reproduce
qcengine info
when this environment is not activatedExpected behavior
I would expect
qcengine
to simply omitpsi4
from the available programs.Additional context
The issue is raised due to the
exc
variable having the following value:which does not include a
stdout
value that can be split.The text was updated successfully, but these errors were encountered: