Move circuit drawer files to `qiskit.visualization.circuit`

[enavarro51]

trafficstars

Summary

Moves circuit drawer files to circuit sub-directory

Details and comments

This PR adds a new circuit sub-directory to the qiskit.visualization directory and moves all circuit drawer modules along with styles and the circuit-related utility functions to the circuit dir.

These changes were also made,

• Two functions in the utils.py file that were only used by state_visualization were moved to that module.
• All other utils functions, except for _trim and matplotlib_close_if_inline which were used by multiple modules, were moved to circuit_utils in the circuit sub-directory.
• The pulse_visualization.py file was removed since it had been deprecated for a long time. @nkanazawa1989 confirmed this was ok.
• Two stub files - circuit_visualization and qcstyle.py - were left in qiskit.visualization which just import from circuit.circuit_visualization and circuit.qcstyle, respectively, for backward compatibility.
• Imports were cleaned up in several modules, including the qiskit.visualization.__init__ module, which included more use of '.' and '..' for directories.

In a future PR, the pulse_v2 files can be moved to the pulse dir. When this is completed and all deprecations delays have passed, the visualization dir will look like this,

visualization
    __init__
    array.py
    bloch.py
    counts_visualization.py
    dag_visualization.py
    exceptions.py
    gate_map.py
    pass_manager_visualization.py
    state_visualization.py
    transition_visualization.py
    utils.py
    -> circuit
    -> pulse
    -> timeline

[qiskit-bot]

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:

• @Qiskit/terra-core
• @enavarro51

[coveralls]

Pull Request Test Coverage Report for Build 3004156037

• 469 of 585 (80.17%) changed or added relevant lines in 13 files are covered.
• 57 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.07%) to 84.298%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/visualization/circuit_visualization.py	0	1	0.0%
qiskit/visualization/qcstyle.py	0	1	0.0%
qiskit/visualization/transition_visualization.py	0	2	0.0%
qiskit/visualization/state_visualization.py	19	25	76.0%
qiskit/visualization/circuit/_utils.py	328	350	93.71%
qiskit/visualization/circuit/circuit_visualization.py	72	106	67.92%
qiskit/visualization/circuit/qcstyle.py	28	78	35.9%
<!--	Total:	469	585

Files with Coverage Reduction	New Missed Lines	%
qiskit/tools/visualization.py	1	0%
qiskit/visualization/circuit_visualization.py	1	0%
qiskit/visualization/qcstyle.py	1	0%
qiskit/visualization/pulse/matplotlib.py	54	0%
<!--	Total:	57

Totals
Change from base Build 2994503304:	-0.07%
Covered Lines:	57808
Relevant Lines:	68576

---

💛 - Coveralls

[1ucian0]

Could you please get another approval since I recently don't maintain the visualization module except for pulse drawer.

Maybe ./.github/CODEOWNERS needs an update too?

Indeed, I think this is a great occation for deprecating pulseV1 and pulseV2->pulse.

[enavarro51]

Luciano's technically correct about the text.py, latex.py and matplotlib.py files - we should put stubs in for them.

I'm not sure I understand the reason for this. Currently this PR creates a stub for circuit_visualization, but only so the user can do from qiskit.visualization import circuit_drawer. I don't know what the use case would be for loading text.py, etc. since the classes in those require that something like the private function _get_layered_instructions from _utils has been called first to get the nodes list. This should only be handled by the user by calling through circuit_drawer assuming no "violation" of private/public.

I can add the stubs in, but it seems practically superfluous.

[enavarro51]

Just to clarify on the stubs. Do we need to support both from qiskit.visualization import text and from qiskit.visualization.text import <something>? Thanks.

[jakelishman]

Yeah, I agree nobody should be using these things. It's mostly that we publish our deprecation policy, and so we really ought to stick with it. It's part of the reason I like to push to make sure internal functions are explicitly marked as private.

The correct way would be for us to ensure from qiskit.visualization.text import ... works, but if that's too much effort, let's just ensure from qiskit.visualization import text works, since that provides a backwards-compatible workaround. I think that's a good enough compromise here.

[enavarro51]

Ok. Allowing from qiskit.visualization import text only requires another __init__ entry, where allowing from qiskit.visualization.text import <something> requires a separate stub text.py file. So I'd vote for the former, since it gives the user a way to still access text without knowing about the circuit directory, but leaves a cleaner visualization directory and fewer deprecations at the next step.

[enavarro51]

Thanks, Jake.