Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

cvxpy not in requirements #312

Closed
nonhermitian opened this issue Dec 10, 2019 · 2 comments · Fixed by #416
Closed

cvxpy not in requirements #312

nonhermitian opened this issue Dec 10, 2019 · 2 comments · Fixed by #416
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nonhermitian
Copy link
Contributor

Informations

  • Qiskit Ignis version: latest
  • Python version:
  • Operating system:

What is the current behavior?

cvxpy is not installed as a requirement because it is not in the setup requirements

Steps to reproduce the problem

What is the expected behavior?

Suggested solutions

@gadial
Copy link
Contributor

gadial commented Dec 11, 2019

Is this considered a bug? I thought this was deliberate - use cvxpy if installed, but if not - revert to scipy. This leaves the choice to the user.

@nonhermitian
Copy link
Contributor Author

I am not sure how the user is supposed to know this though. Unless one looks at the dev requirements or has the code raise the exception, there is no mention of this anywhere.

@chriseclectic chriseclectic added this to the 0.3.1 milestone May 19, 2020
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this issue Jun 2, 2020
Ignis has 2 optional dependencies which are not required to use the
core functionality but are required to use certain features. These
weren't always clearly documented or exposed so it could potentially
come as a surprise to end users. This commit makes 3 changes around the
use of optional dependencies. First it updates the cvx error message to
more clearly explain the options. Then it adds an extras section to the
setup.py so that users can install the optional dependencies when they
install ignis. Finally a section is added to the readme to document the
optional dependencies.

Fixes qiskit-community#312
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this issue Jun 22, 2020
This commit adds a new ci job for running ignis tests without any
optional dependencies. There are several optional dependencies which do
not not always get installed with ignis. They are needed to enable
optional features but shouldn't be required, we've had a slew of recent
bugs around accidentally requiring these optional dependencies (see
issues qiskit-community#429, qiskit-community#422, and qiskit-community#312). None of these were caught in CI because we
always install all optional dependencies in CI test jobs. By adding a
new job which explicitly installs the bare minimum we're emulating what
a user does when they install just ignis.

As part of this a missing dependency was added to the requirements list.
Ignis has a hard dependency on scikit learn for the measurement
discriminators, but this was never explicitly listed. This was never
caught because the test jobs were always installing it.
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this issue Jun 22, 2020
This commit adds a new ci job for running ignis tests without any
optional dependencies. There are several optional dependencies which do
not not always get installed with ignis. They are needed to enable
optional features but shouldn't be required, we've had a slew of recent
bugs around accidentally requiring these optional dependencies (see
issues qiskit-community#429, qiskit-community#422, and qiskit-community#312). None of these were caught in CI because we
always install all optional dependencies in CI test jobs. By adding a
new job which explicitly installs the bare minimum we're emulating what
a user does when they install just ignis.

As part of this a missing dependency was added to the requirements list.
Ignis has a hard dependency on scikit learn for the measurement
discriminators, but this was never explicitly listed. This was never
caught because the test jobs were always installing it.
chriseclectic pushed a commit that referenced this issue Jun 22, 2020
…ies (#436)

* Add scikit-learn dependency and add CI job without optional deps

This commit adds a new ci job for running ignis tests without any
optional dependencies. There are several optional dependencies which do
not not always get installed with ignis. They are needed to enable
optional features but shouldn't be required, we've had a slew of recent
bugs around accidentally requiring these optional dependencies (see
issues #429, #422, and #312). None of these were caught in CI because we
always install all optional dependencies in CI test jobs. By adding a
new job which explicitly installs the bare minimum we're emulating what
a user does when they install just ignis.

As part of this a missing dependency was added to the requirements list.
Ignis has a hard dependency on scikit learn for the measurement
discriminators, but this was never explicitly listed. This was never
caught because the test jobs were always installing it.

* Don't install cvxopt in no-opt job

* Add job name
mtreinish added a commit to mtreinish/qiskit-ignis that referenced this issue Jun 22, 2020
…ies (qiskit-community#436)

* Add scikit-learn dependency and add CI job without optional deps

This commit adds a new ci job for running ignis tests without any
optional dependencies. There are several optional dependencies which do
not not always get installed with ignis. They are needed to enable
optional features but shouldn't be required, we've had a slew of recent
bugs around accidentally requiring these optional dependencies (see
issues qiskit-community#429, qiskit-community#422, and qiskit-community#312). None of these were caught in CI because we
always install all optional dependencies in CI test jobs. By adding a
new job which explicitly installs the bare minimum we're emulating what
a user does when they install just ignis.

As part of this a missing dependency was added to the requirements list.
Ignis has a hard dependency on scikit learn for the measurement
discriminators, but this was never explicitly listed. This was never
caught because the test jobs were always installing it.

* Don't install cvxopt in no-opt job

* Add job name

* Update tox.ini to point to remove master terra for stable branch

(cherry picked from commit f0b68e4)
chriseclectic pushed a commit that referenced this issue Jun 22, 2020
…ies (#436) (#439)

* Add scikit-learn dependency and add CI job without optional deps

This commit adds a new ci job for running ignis tests without any
optional dependencies. There are several optional dependencies which do
not not always get installed with ignis. They are needed to enable
optional features but shouldn't be required, we've had a slew of recent
bugs around accidentally requiring these optional dependencies (see
issues #429, #422, and #312). None of these were caught in CI because we
always install all optional dependencies in CI test jobs. By adding a
new job which explicitly installs the bare minimum we're emulating what
a user does when they install just ignis.

As part of this a missing dependency was added to the requirements list.
Ignis has a hard dependency on scikit learn for the measurement
discriminators, but this was never explicitly listed. This was never
caught because the test jobs were always installing it.

* Don't install cvxopt in no-opt job

* Add job name

* Update tox.ini to point to remove master terra for stable branch

(cherry picked from commit f0b68e4)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants