Ensure that qchem is imported lazily #1962
Conversation
|
Hello. You may have forgotten to update the changelog!
|
josh146
added
the
review-ready 👌
| f"The {name} plugin requires PennyLane versions {plugin_device_class.pennylane_requires}, " | ||
| f"however PennyLane version {__version__} is installed." |
There was a problem hiding this comment.
This is because pylint was complaining
|
Thumbs up! |
Codecov Report
@@ Coverage Diff @@
## master #1962 +/- ##
==========================================
+ Coverage 96.90% 98.81% +1.91%
==========================================
Files 226 226
Lines 17346 17348 +2
==========================================
+ Hits 16809 17143 +334
+ Misses 537 205 -332
Continue to review full report at Codecov.
|
| _qchem = None | ||
|
|
||
|
|
||
| def __getattr__(name): |
There was a problem hiding this comment.
I performed VQE calculations in the following cases, everything seems to work as expected.
A. PennyLane-QChem is installed,
-
`from pennylane import qchem` works fine -
`qml.qchem.excitations(2, 4)` without importing qchem works fine
B. PennyLane-QChem is not installed
-
`from pennylane import qchem`: does not complain -
`qml.qchem.excitations(2, 4)` with/without importing qchem: asks for installing qchem
|
Thanks Soran for checking! |
|
By the way: Are you aware of https://github.com/mnmelo/lazy_import ? Just stumbled across it and it looks pretty amazing. |
|
Nice! I actually wasn't aware. Although it seems to be unmaintained for 4 years now... I imagine the ability to set module level (Although, its testing module looks quite nice) |
|
|
||
|
|
||
| # add everything as long as it's not a module and not prefixed with _ | ||
| _all = sorted( |
There was a problem hiding this comment.
How come we're sorting here?
There was a problem hiding this comment.
Good question @antalszava.. I do not remember why I am sorting this.
| if entry.name == "OpenFermion": | ||
| _qchem = entry.load() | ||
|
|
||
| if _qchem is None: | ||
| raise ImportError( |
There was a problem hiding this comment.
This seems to check if OpenFermion was found as an entry. What if we have PennyLane-QChem installed, but OpenFermion uninstalled (e.g., first Qchem was installed, but then OpenFermion got manually uninstalled)? We'd get the ImportError, however, it refers to QChem not Openfermion, correct?
There was a problem hiding this comment.
oh this is an historical accident 😆 The entry point for qchem is named "OpenFermion", since we initially envisioned qchem as an openfermion plugin.
| def __getattr__(name): | ||
| """Ensure that the qchem module is imported lazily""" | ||
| if name == "qchem": | ||
| global _qchem # pylint: disable=global-statement |
There was a problem hiding this comment.
For my understanding: how come we have to define _qchem as global here and not on line 322?
There was a problem hiding this comment.
Ah, _qchem is an existing module level variable, but the way Python scope works is as follows:
a = 5
def f():
# No assignment; Python assumes
# a refers to the global variable a above.
print(a)
def g():
# a is assigned to within the function;
# Python assigns it with a scope *local*
# to the function, independent of the global
# a above.
a = 7
print(a)If we wish g() to assign value to the global variable a above, we need to then explicitly state this using the global keyword.
Context:
pennylane.qchemis currently imported when PennyLane is imported, if it is available. If not,pennylane.qcheminstead points to a dummyNestedAttributeErrorclass, which raises an import error if or when the user attempts to accessqml.qchem.Description of the Change: With Python 3.7, module level
__getattr__and__dir__special methods can now be defined. This allows us to modify this system, to ensure qchem is only imported when accessed.Benefits: Potential speed up in import time.
For example:
Possible Drawbacks: n/a
Related GitHub Issues: Closes #1961