Skip to content
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
merged 12 commits into from
Dec 26, 2018

Conversation

diego-plan9
Copy link
Member

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:

  • revising .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 redefined compile import.
  • fixed 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:

  • moved the 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.
  • move the version gathering to its own module as well (qiskit.version), as its placement at the bottom of qiskit.__init__.py was causing another cycle in the backends.
  • other cyclic imports were solved by importing the specific file or module where the member was defined rather than directly from top-level qiskit. Special prunning was made in the qiskit.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 the from_qasm_str() function, that should be taken care of in a follow-up PR.

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.
@1ucian0 1ucian0 merged commit 65ba65d into Qiskit:master Dec 26, 2018
@diego-plan9 diego-plan9 deleted the feature/pylint-upgrade-cyclic branch December 26, 2018 16:01
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update pylint to a more recent version
2 participants