-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update pylint to recent version, solve cyclic dependencies #1600
Merged
1ucian0
merged 12 commits into
Qiskit:master
from
diego-plan9:feature/pylint-upgrade-cyclic
Dec 26, 2018
Merged
Update pylint to recent version, solve cyclic dependencies #1600
1ucian0
merged 12 commits into
Qiskit:master
from
diego-plan9:feature/pylint-upgrade-cyclic
Dec 26, 2018
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
Disable pylint checks: * unnecessary-pass: allow for methods with just "pass", for clarity * no-else-return: relax "elif" after a clause with a return
Move the reading of Terra version to its own file, avoiding cyclic import during at `qiskit/providers/basebackend.py`.
Revise the imports of `QiskitError` in order to break cyclic dependencies.
Move the pylint disable to an inner-most level for QuantumCircuit. There is still an import cycle (qiskit.circuit.quantumcircuit -> qiskit.converters.ast_to_dag) due to circuit.Measure and circuit.Reset being part of the module and the dynamic extension mechanism.
Revise the Exceptions throughout the codebase, containing them into specific `.exceptions` modules.
Update the `qiskit.transpiler` module imports to avoid cyclic import, after rebasing against master.
diego-plan9
requested review from
1ucian0,
ajavadia,
atilag,
delapuente,
ewinston,
jaygambetta,
mtreinish,
nonhermitian and
taalexander
as code owners
December 26, 2018 13:45
1ucian0
approved these changes
Dec 26, 2018
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* Bump pylint version * Disable pylint unnecessary-pass, no-else-return Disable pylint checks: * unnecessary-pass: allow for methods with just "pass", for clarity * no-else-return: relax "elif" after a clause with a return * Fix linter: useless-object-inheritance * Fix new linting errors * Move version to its own file Move the reading of Terra version to its own file, avoiding cyclic import during at `qiskit/providers/basebackend.py`. * Prefer importing QiskitError from submodule Revise the imports of `QiskitError` in order to break cyclic dependencies. * Break more cyclic imports * Break more cyclic imports, redux * Update cyclic import in QuantumCircuit Move the pylint disable to an inner-most level for QuantumCircuit. There is still an import cycle (qiskit.circuit.quantumcircuit -> qiskit.converters.ast_to_dag) due to circuit.Measure and circuit.Reset being part of the module and the dynamic extension mechanism. * Unify extension handling in own modules Revise the Exceptions throughout the codebase, containing them into specific `.exceptions` modules. * Update passes imports after rebasing Update the `qiskit.transpiler` module imports to avoid cyclic import, after rebasing against master. * Update .pylintrc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix #1597
Summary
Bump the
pylint
version to the latest release (2.2.x
), in order to take advantage of new functionality and better compatibility. This resulted in a rather noisy PR - checking each commit individually will probably shed some light, as it encompasses:.pylintrc
, revising the disables and tweaking minor things. A couple of checks have been relaxed, and a basic check for the docstring style has been included along with a touch for the redefinedcompile
import.useless-object-inheritance
warnings (class Foo(object): -> class Foo:
) and other minor warnings in the new version.A numbre of cyclic import warnings were raised with the new version, so additionally:
Exception
subclasses to their own modules (qiskit.foo.exceptions
), preferring importing from the submodules directly. This breaks a bunch of cycles, and while the renaming of the module was not part of the linting upgrade, it hopefully makes the code more consistent.qiskit.version
), as its placement at the bottom ofqiskit.__init__.py
was causing another cycle in the backends.qiskit
. Special prunning was made in theqiskit.transpiler
module, aiming for respecting the original hierarchy as much as possible - further re-organizing that particular module in another PR might be a good idea, as the import graph is a bit tricky to follow.Details and comments
There is still a big cyclic import caused by
qiskit.circuit.quantumcircuit -> qiskit.converters.ast_to_dag
, mostly due to thefrom_qasm_str()
function, that should be taken care of in a follow-up PR.