Cleaning the code base #290
Conversation
Mayank447
force-pushed
the
master
branch
2 times, most recently
from
cc742e7 to
6ba9555
Compare
|
Nice thanks for doing this. I will have a look, but try to avoid exploding the PR. Let's keep the size as small as possible (At the moment it seems ok, maybe move the qasm and circuit drawer change to a different one.). I just noticed that in the last PR the test_qiskit was mistakely commented out in the end, which I missed. Also to avoid having to manually slient the qiskit test, I would recomment to create a different branch instead of using master and create a new environment, either using conda or virtual env. So the development won't affect your existing environment. And you can have multiple version at the sametime. This is good practice in general. |
|
Sure, I will keep this in mind. I will move qasm, circuit to a different PR, those refactor are pretty huge. |
…ule import structure
…dule import structure
…le import structure
…odule import structure
…cture for remaining files
|
@BoxiLi I have removed the commits for refactoring qasm and circuit draw, I will open separate individual PRs for those. This PR is ready for review |
|
|
||
| from .circuit import * | ||
| from .circuitsimulator import * | ||
| from ..operations import Gate, Measurement | ||
| from .circuitsimulator import CircuitResult, CircuitSimulator | ||
| from .circuit import QubitCircuit | ||
| from qutip_qip.operations import Gate, Measurement | ||
|
|
||
|
|
There was a problem hiding this comment.
I think for the __init__.py files, it is fine to use the star importation. We already defined __all__.py in all the .py files. This simplifies the development. Otherwise one have to always rember to update __init__.py when a new function is added.
There was a problem hiding this comment.
And is there a reason that relative import is kept here but removed elsewhere? I don't have strong preference but it should be consistent.
|
|
||
|
|
||
| __all__ = [ | ||
| "CircuitSimulator", | ||
| "CircuitResult", | ||
| "QubitCircuit", | ||
| ] |
| from ..compiler import GateCompiler, Instruction | ||
|
|
||
|
|
||
| __all__ = ["SCQubitsCompiler"] |
There was a problem hiding this comment.
Why is __all__ removed here?
I think typically we do in qutip family package is to keep this in all .py files. Not __init__.py. Unless there is some advantages?
There was a problem hiding this comment.
This applies to many other files.
2 participants
qutip_qip.circuit import QubitCircuit) instead of relative..circuit import QubitCircuit. This helps when files are moved around or refactored.qasm.pyhas been broken down into a module since the no. of lines of code was over 1300 (the external import structure remains the same), separatedModelinto a separate file, renderers were refactored into a module namedrawundersrc/circuits@BoxiLi This PR isn't fully finished yet, its still a draft. But can you review it please since the code changes especially refactor is pretty huge.
Update: qasm.py refactoring and circuit draw would be done in separate PR.