Skip to content

Conversation

jakelishman
Copy link
Member

Summary

This pylint rule has a tendency to be overzealous when we're dealing with optional packages that we don't require to be installed, with compiled modules, and with packages which dynamically mutate the Python path look-up. Overall, it was giving far more false positives than anything else, and a failure-to-import should be abundantly obvious from a standard test run, so we shouldn't be effectively losing coverage.

Details and comments

With the work in #8967 moving to do more testing of the no-optionals paths, and to make a cleaner split between "optional package" and "development package", the additional pain pylint is causing with its overzealous import-error makes it more worthwhile to remove now. Python importability cannot generally be determined statically, despite what pylint tries.

This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.
@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog labels Oct 20, 2022
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@coveralls
Copy link

coveralls commented Oct 20, 2022

Pull Request Test Coverage Report for Build 3299725393

  • 11 of 14 (78.57%) changed or added relevant lines in 14 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.004%) to 84.419%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/quantum_info/operators/measures.py 0 1 0.0%
qiskit/transpiler/passes/optimization/_gate_extension.py 0 1 0.0%
qiskit/utils/optionals.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
qiskit/tools/jupyter/init.py 1 0%
qiskit/tools/jupyter/job_watcher.py 2 0%
Totals Coverage Status
Change from base Build 3291944382: 0.004%
Covered Lines: 61708
Relevant Lines: 73097

💛 - Coveralls

@mergify mergify bot merged commit 6f8d2ce into Qiskit:main Oct 21, 2022
@jakelishman jakelishman deleted the disable-import-error branch October 21, 2022 20:02
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms that referenced this pull request Jul 27, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Oct 12, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Dec 8, 2023
This pylint rule has a tendency to be overzealous when we're dealing
with optional packages that we don't require to be installed, with
compiled modules, and with packages which dynamically mutate the Python
path look-up.  Overall, it was giving far more false positives than
anything else, and a failure-to-import should be abundantly obvious from
a standard test run, so we shouldn't be _effectively_ losing coverage.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants