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

Refactor OpenEye checks to only warn if installed and unlicensed #1426

Merged
merged 4 commits into from
Oct 14, 2022

Conversation

mattwthompson
Copy link
Member

@mattwthompson mattwthompson commented Oct 13, 2022

Closes #1425 and possibly other issues

The result of high-level imports, i.e. from openff.toolkit import Molecule, for the cases of OpenEye Toolkits being installed and/or licensed are as follows:

Installed Not installed
Licensed Nothing Nothing
Not licensed A versbose warning Nothing

Said warning, for various cases of oechem not finding the environment variable or file:

>>> from openff.toolkit import Molecule
LICENSE: Could not open license file "oe_license.txt" in local directory
LICENSE: N.B. OE_LICENSE environment variable is not set
LICENSE: N.B. OE_DIR environment variable is not set
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
The OpenEye Toolkits are found to be installed but not licensed and therefore will not be used.
The OpenEye Toolkits require a (free for academics) license which can be found at https://docs.eyesopen.com/toolkits/python/quickstart-python/install.html

More thoroughly, showing

  • No warning when license exists and is set up properly
  • Warning when openeye-toolkits is installed and OE_LICENSE is set but file does not exist
  • Warning when openeye-toolkits is installed and OE_LICENSE is not set
  • No warning when openeye-toolkits is not installed and license is not set up
  • No warning when openeye-toolkits is not installed and license is set up properly

This covers everything I can think of.

(openff-interchange-env) [openff-toolkit] wc -l ~/.oe_license.txt && echo $OE_LICENSE
     197 /Users/mattthompson/.oe_license.txt
/Users/mattthompson/.oe_license.txt
(openff-interchange-env) [openff-toolkit] python -c "from openff.toolkit import Molecule"
(openff-interchange-env) [openff-toolkit] mv ~/.oe_license.txt ~/.foo_license.txt
(openff-interchange-env) [openff-toolkit] python -c "from openff.toolkit import Molecule"
LICENSE: Could not open license file specified by OE_LICENSE environment variable "/Users/mattthompson/.oe_license.txt"
LICENSE: Could not open license file "oe_license.txt" in local directory
LICENSE: N.B. OE_DIR environment variable is not set
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
The OpenEye Toolkits are found to be installed but not licensed and therefore will not be used.
The OpenEye Toolkits require a (free for academics) license which can be found at https://docs.eyesopen.com/toolkits/python/quickstart-python/install.html
(openff-interchange-env) [openff-toolkit] unset OE_LICENSE          10:42:44  ☁  quieter-openeye-warning ☂
(openff-interchange-env) [openff-toolkit] python -c "from openff.toolkit import Molecule"
LICENSE: Could not open license file "oe_license.txt" in local directory
LICENSE: N.B. OE_LICENSE environment variable is not set
LICENSE: N.B. OE_DIR environment variable is not set
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
LICENSE: No product keys!
The OpenEye Toolkits are found to be installed but not licensed and therefore will not be used.
The OpenEye Toolkits require a (free for academics) license which can be found at https://docs.eyesopen.com/toolkits/python/quickstart-python/install.html
(openff-interchange-env) [openff-toolkit] CONDA_SUBDIR=osx-64 mamba uninstall openeye-toolkits -yq

                  __    __    __    __
                 /  \  /  \  /  \  /  \
                /    \/    \/    \/    \
███████████████/  /██/  /██/  /██/  /████████████████████████
              /  / \   / \   / \   / \  \____
             /  /   \_/   \_/   \_/   \    o \__,
            / _/                       \_____/  `
            |/
        ███╗   ███╗ █████╗ ███╗   ███╗██████╗  █████╗
        ████╗ ████║██╔══██╗████╗ ████║██╔══██╗██╔══██╗
        ██╔████╔██║███████║██╔████╔██║██████╔╝███████║
        ██║╚██╔╝██║██╔══██║██║╚██╔╝██║██╔══██╗██╔══██║
        ██║ ╚═╝ ██║██║  ██║██║ ╚═╝ ██║██████╔╝██║  ██║
        ╚═╝     ╚═╝╚═╝  ╚═╝╚═╝     ╚═╝╚═════╝ ╚═╝  ╚═╝

        mamba (0.25.0) supported by @QuantStack

        GitHub:  https://github.com/mamba-org/mamba
        Twitter: https://twitter.com/QuantStack

█████████████████████████████████████████████████████████████

  Package              Version  Build   Channel     Size
──────────────────────────────────────────────────────────
  Remove:
──────────────────────────────────────────────────────────

  - openeye-toolkits  2022.1.1  py38_0  openeye

  Summary:

  Remove: 1 packages

  Total download: 0 B

──────────────────────────────────────────────────────────

Preparing transaction: ...working... done
Verifying transaction: ...working... done
Executing transaction: ...working... done
(openff-interchange-env) [openff-toolkit] python -c "from openff.toolkit import Molecule"
(openff-interchange-env) [openff-toolkit] mv ~/.foo_license.txt ~/.oe_license.txt
(openff-interchange-env) [openff-toolkit] export OE_LICENSE=/Users/mattthompson/.oe_license.txt
(openff-interchange-env) [openff-toolkit] python -c "from openff.toolkit import Molecule"

@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #1426 (0cdf4ef) into main (5f7d0d1) will decrease coverage by 0.16%.
The diff coverage is 68.75%.

Additional details and impacted files

@richardjgowers
Copy link
Contributor

The matrix of options here looks great.

@mikemhenry
Copy link
Collaborator

This is great! I think we've talked about this for a year now, so it is great to see it implemented.

My only thought is this case:

  • No warning when openeye-toolkits is not installed and license is set up properly

This strikes me as a warning could be helpful, since this seems like the case where a user has licensed openeye, but forgot to install it.

@mattwthompson
Copy link
Member Author

I agree, "were my conformers generated with Omega / is AM1-BCC being run with QUACPAC or sqm" are common questions and user pain points. However the difficulty is that the license check is done by accessing the API directly; we could attempt mirror the functionality but that gets a little complex and easy to fall out of sync if anything changes upstream.

There are other ways to check what's installed and actually used (ToolkitRegistry.registered_toolkits and setting the logging level to INFO) though I'll admit they could be made more accessible to users, especially beginners. Documentation might help, though I'm not sure where it would go. Maybe @Yoshanuikabundi has thoughts here - might be out of scope for this PR though.

@mattwthompson mattwthompson marked this pull request as ready for review October 13, 2022 19:21
Copy link
Collaborator

@Yoshanuikabundi Yoshanuikabundi left a comment

Choose a reason for hiding this comment

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

I love this change, the Toolkit has tonnes of warnings at the moment and it's nice to reduce that. I've found a couple of blocking changes where the errors are not rendered correctly, and one or two ideas of simplifications.

The first thing I thought when I saw this PR is that we should document how to check which toolkits are available. I think "Optional dependencies (toolkits)" section of the installation guide is a natural place for this - maybe something like:

All available toolkits are automatically registered in the 
`GLOBAL_TOOLKIT_REGISTRY`. The available toolkits and their 
versions can be inspected through the `registered_toolkit_versions`
dictionary:

```python
from openff.toolkit import GLOBAL_TOOLKIT_REGISTRY
print(GLOBAL_TOOLKIT_REGISTRY.registered_toolkit_versions)
```

openff/toolkit/utils/openeye_wrapper.py Outdated Show resolved Hide resolved
openff/toolkit/utils/openeye_wrapper.py Outdated Show resolved Hide resolved
openff/toolkit/utils/openeye_wrapper.py Outdated Show resolved Hide resolved
openff/toolkit/utils/openeye_wrapper.py Outdated Show resolved Hide resolved
openff/toolkit/utils/toolkit_registry.py Outdated Show resolved Hide resolved
@mattwthompson
Copy link
Member Author

Thanks everybody for the feedback!

@mattwthompson mattwthompson merged commit 67d2261 into main Oct 14, 2022
@mattwthompson mattwthompson deleted the quieter-openeye-warning branch October 14, 2022 15:00
@mattwthompson mattwthompson changed the title Refactor OpenEye checks to only warn if installed and unlicesed Refactor OpenEye checks to only warn if installed and unlicensed Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to silence the toolkit warning
4 participants