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

Clean up text drawer compression issues #8588

Merged
merged 30 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b7a3b79
Add barrier labels to mpl and text drawers
enavarro51 Aug 2, 2022
096c01a
Release note
enavarro51 Aug 2, 2022
a47b989
Merge branch 'main' into show_barrier_label
enavarro51 Aug 3, 2022
2ae33d9
Add import
enavarro51 Aug 3, 2022
389ce5b
Merge branch 'show_barrier_label' of github.com:enavarro51/qiskit-ter…
enavarro51 Aug 3, 2022
c84a44d
Lint
enavarro51 Aug 3, 2022
789f21b
Add barrier labels to latex drawer
enavarro51 Aug 4, 2022
ae09894
Merge branch 'main' into show_barrier_label
enavarro51 Aug 4, 2022
b26a3ed
Remove utils changes
enavarro51 Aug 7, 2022
cec670e
Merge branch 'main' into show_barrier_label
enavarro51 Aug 7, 2022
f79d27a
Cleanup
enavarro51 Aug 7, 2022
0e9b49e
Merge branch 'main' into show_barrier_label
enavarro51 Aug 11, 2022
e23aaaa
Fix merge conflict
enavarro51 Aug 11, 2022
959b9a6
Merge branch 'show_barrier_label' of github.com:enavarro51/qiskit-ter…
enavarro51 Aug 11, 2022
af5377d
Merge branch 'main' into show_barrier_label
enavarro51 Aug 12, 2022
7b3b92b
Lint
enavarro51 Aug 12, 2022
b8349db
Merge branch 'show_barrier_label' of github.com:enavarro51/qiskit-ter…
enavarro51 Aug 12, 2022
1a17e25
Remove label property for barriers and snapshots and increase mpl lab…
enavarro51 Aug 17, 2022
99d1263
First steps
enavarro51 Aug 18, 2022
01b9179
Fix vert spacing with text between lines
enavarro51 Aug 19, 2022
500d9b2
Merge branch 'main' into fix_vert_compress_text
enavarro51 Aug 19, 2022
4005ec2
Fix low compress connections and change tests
enavarro51 Aug 19, 2022
4512eeb
Add tests
enavarro51 Aug 20, 2022
1788218
Additonal test
enavarro51 Aug 20, 2022
755a3a4
Lint
enavarro51 Aug 20, 2022
b463348
Merge branch 'main' into fix_vert_compress_text
enavarro51 Sep 2, 2022
8062509
Add back test
enavarro51 Sep 2, 2022
7471981
Release note
enavarro51 Sep 2, 2022
632f102
Fix typo in release note
jakelishman Sep 2, 2022
84d280a
Merge branch 'main' into fix_vert_compress_text
mergify[bot] Sep 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions qiskit/visualization/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -867,9 +867,7 @@ def should_compress(self, top_line, bot_line):
for top, bot in zip(top_line, bot_line):
if top in ["┴", "╨"] and bot in ["┬", "╥"]:
return False
for line in (bot_line, top_line):
no_spaces = line.replace(" ", "")
if len(no_spaces) > 0 and all(c.isalpha() or c.isnumeric() for c in no_spaces):
if (top.isalnum() and bot != " ") or (bot.isalnum() and top != " "):
return False
return True

Expand Down Expand Up @@ -959,6 +957,8 @@ def merge_lines(top, bot, icod="top"):
ret += "╫"
elif topc in "║╫╬" and botc in " ":
ret += "║"
elif topc in "│┼╪" and botc in " ":
ret += "│"
elif topc == "└" and botc == "┌" and icod == "top":
ret += "├"
elif topc == "┘" and botc == "┐":
Expand Down Expand Up @@ -1518,8 +1518,10 @@ def connect_with(self, wire_char):
wire_char = "║"
if index == 0 and len(affected_bits) > 1:
affected_bit.connect(wire_char, ["bot"])
else:
elif index == len(affected_bits) - 1:
affected_bit.connect(wire_char, ["top"])
else:
affected_bit.connect(wire_char, ["bot", "top"])
else:
if index == 0:
affected_bit.connect(wire_char, ["bot"])
Expand Down
111 changes: 101 additions & 10 deletions test/python/visualization/test_circuit_text_drawer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1688,22 +1688,22 @@ def test_control_gate_label_with_cond_2_med(self):
See https://github.com/Qiskit/qiskit-terra/issues/4361"""
expected = "\n".join(
[
" ┌──────┐",
"q_0: |0>┤ my h ├",
" └──┬───┘",
"q_1: |0>───────",
" my ch ",
" ",
" c: 0 ═══════",
" 0x1 ",
" ┌──────┐ ",
"q_0: |0>──┤ my h ├",
" └──┬───┘ ",
"q_1: |0>─────■─────",
" my ctrl-h ",
" ",
" c: 0 ═════■═════",
" 0x1 ",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we revert this test case? The behaviour that appears with this PR and this test is probably something we want to make a positive decision on, and potentially test both it and your new form (with the space not aligned).

For clarity: with this PR, the original test produces

        ┌──────┐
q_0: |0>┤ my h ├
        └──┬───┘
q_1: |0>───■────
         my║ch
   c: 0 ═══■════
          0x1

instead. I feel like that (^) behaviour is probably actually suitable for vertical_compression=medium - it compresses vertically when there's a "space" so nothing visible gets erased, whereas "high" always compresses and "low" never does.

Perhaps leave the test as-is, and add your new one as a separate case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I was hoping nobody would notice that. It works visually either way and it's probably a pretty rare case. I'll add the old test back with the new one.

]
)

qr = QuantumRegister(2, "q")
cr = ClassicalRegister(1, "c")
circ = QuantumCircuit(qr, cr)
hgate = HGate(label="my h")
controlh = hgate.control(label="my ch").c_if(cr, 1)
controlh = hgate.control(label="my ctrl-h").c_if(cr, 1)
circ.append(controlh, [1, 0])

self.assertEqual(str(_text_circuit_drawer(circ, vertical_compression="medium")), expected)
Expand Down Expand Up @@ -2046,7 +2046,6 @@ def test_text_conditional_1(self):
" └─╥─┘└─╥─┘",
"c0: 0 ══■════╬══",
" 0x1 ║ ",
" ║ ",
"c1: 0 ═══════■══",
" 0x1 ",
]
Expand Down Expand Up @@ -2148,6 +2147,98 @@ def test_text_measure_with_spaces_bundle(self):
expected,
)

def test_text_barrier_med_compress_1(self):
"""Medium vertical compression avoids connection break."""
circuit = QuantumCircuit(4)
circuit.cx(1, 3)
circuit.x(1)
circuit.barrier((2, 3), label="Bar 1")

expected = "\n".join(
[
" ",
"q_0: |0>────────────",
" ┌───┐ ",
"q_1: |0>──■───┤ X ├─",
" │ └───┘ ",
" │ Bar 1 ",
"q_2: |0>──┼─────░───",
" ┌─┴─┐ ░ ",
"q_3: |0>┤ X ├───░───",
" └───┘ ░ ",
]
)

self.assertEqual(
str(_text_circuit_drawer(circuit, vertical_compression="medium", cregbundle=False)),
expected,
)

def test_text_barrier_med_compress_2(self):
"""Medium vertical compression avoids overprint."""
circuit = QuantumCircuit(4)
circuit.barrier((0, 1, 2), label="a")
circuit.cx(1, 3)
circuit.x(1)
circuit.barrier((2, 3), label="Bar 1")

expected = "\n".join(
[
" a ",
"q_0: |0>─░─────────────",
" ░ ┌───┐ ",
"q_1: |0>─░───■───┤ X ├─",
" ░ │ └───┘ ",
" ░ │ Bar 1 ",
"q_2: |0>─░───┼─────░───",
" ░ ┌─┴─┐ ░ ",
"q_3: |0>───┤ X ├───░───",
" └───┘ ░ ",
]
)

self.assertEqual(
str(_text_circuit_drawer(circuit, vertical_compression="medium", cregbundle=False)),
expected,
)

def test_text_barrier_med_compress_3(self):
"""Medium vertical compression avoids conditional connection break."""
qr = QuantumRegister(1, "qr")
qc1 = ClassicalRegister(3, "cr")
qc2 = ClassicalRegister(1, "cr2")
circuit = QuantumCircuit(qr, qc1, qc2)
circuit.x(0).c_if(qc1, 3)
circuit.x(0).c_if(qc2[0], 1)

expected = "\n".join(
[
" ┌───┐┌───┐",
" qr: |0>┤ X ├┤ X ├",
" └─╥─┘└─╥─┘",
"cr_0: 0 ══■════╬══",
" ║ ║ ",
"cr_2: 0 ══o════╬══",
" ║ ║ ",
" cr2: 0 ══╬════■══",
" ║ ",
"cr_1: 0 ══■═══════",
" 0x3 ",
]
)

self.assertEqual(
str(
_text_circuit_drawer(
circuit,
vertical_compression="medium",
wire_order=[0, 1, 3, 4, 2],
cregbundle=False,
)
),
expected,
)


class TestTextConditional(QiskitTestCase):
"""Gates with conditionals"""
Expand Down