This repository has been archived by the owner on Jun 12, 2023. It is now read-only.
Add scikit-learn dependency and add CI job without optional dependencies #436
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mtreinish
added
Changelog: Bugfix
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
force-pushed
the
add-no-opt-job
branch
from
54f9784 to
0fea1e2
Compare
mtreinish
changed the title
chriseclectic
approved these changes
Jun 22, 2020
mtreinish
added a commit
to mtreinish/qiskit-ignis
that referenced
this pull request
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 pull request
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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
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.
Details and comments