Skip to content

Conversation

@rajibchakravorty
Copy link
Contributor

Lazy load/Conditionally loads 3rd party libraries : qiskit, pyquil and cirq.

Based on our discussion, change made from the draft PR earlier:

  1. opencontrols still will have all the external libraries as dependency - this implies that those libraries are still installed when user installs opencontrols as opposed to user installing those separately.

  2. Removed all package load/import error checks. This is because the 3-rd party libraries are still part of the dependency list in `opencontrols'. In that case, there is no change in behaviour whether the libraries are imported at the top of the module or inside a method within the module. The separation of.

Notes

  1. Added pylint: disable=too-many-variablesin a couple of places - the solution is known; wanted to concentrate on library loading in this PR [that and a couple of other minor changes will be made in successive PR]
  2. When importing cirq modules in this fashion from cirq import .... instead of import cirq followed by cirq...., pylint somehow complains and could not find a solution. Any suggestion is welcome there. Not a major issue, but somehow breaks the pattern compared to what is done in other libraries.

Copy link
Contributor

@charmasaur charmasaur left a comment

Choose a reason for hiding this comment

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

When you say "pylint somehow complains and could not find a solution" regarding the cirq imports, what exactly is it complaining about?

In any case, this seems fine to me. Don't get me started on the "too many locals" check -- I think it's pretty silly, so fine to disable.

@michaelhush
Copy link
Contributor

I'm going to remove myself as a reviewer, as I don't understand what the purpose of this PR is anymore.

If you haven't removed the qiskit, cirq and pyquil as dependencies in the install. Then I wouldn't call this lazy loading, indeed I don't really know what the point of this is.

Nevertheless if this is somehow helpful for getting this dependency review fixed in the short term then I'll leave this up to @charmasaur and @rajibchakravorty if you want to pull it in.

In order to keep track of the other minor issues that have popped up after this PR, I would recommend you make tasks in back log on Jira @rajibchakravorty.

@michaelhush michaelhush removed their request for review October 29, 2019 09:45
@rajibchakravorty rajibchakravorty merged commit 4b6a714 into master Oct 29, 2019
@rajibchakravorty rajibchakravorty deleted the lazy-loading branch October 29, 2019 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants