Address review comments

brainglobe_utils/qtpy/collapsible_widget.py

@@ -21,10 +21,6 @@ class CollapsibleWidget(QCollapsible):
         The title of the CollapsibleWidget.
     parent : QWidget or None, optional
         The parent widget.
-    expanded_icon : str or None, optional
-        The ASCII symbol for the expanded state.
-    collapsed_icon : str or None, optional
-        The ASCII symbol for the collapsed state.
     """
 
     toggled_signal_with_self = Signal(QCollapsible, bool)
@@ -33,8 +29,6 @@ def __init__(
         self,
         title: str = "",
         parent: Optional[QWidget] = None,
-        expanded_icon: Optional[str] = "▲",
-        collapsed_icon: Optional[str] = "▼",
     ):
         """
         Initializes a new CollapsibleWidget instance.
@@ -45,12 +39,8 @@ def __init__(
             The title of the CollapsibleWidget.
         parent : QWidget or None, optional
             The parent widget.
-        expanded_icon : str or None, optional
-            The icon for the expanded state.
-        collapsed_icon : str or None, optional
-            The icon for the collapsed state.
         """
-        super().__init__(title, parent, expanded_icon, collapsed_icon)
+        super().__init__(title, parent)
         self.currently_expanded = False
 
         self.toggled.connect(self._on_toggle)

tests/tests/test_unit/test_qtpy/test_collapsible_widget.py

@@ -6,14 +6,12 @@
     CollapsibleWidgetContainer,
 )
 
-widget_title = "Title"
+WIDGET_TITLE = "Title"
 
 
 @pytest.fixture(scope="class")
 def collapsible_widget() -> CollapsibleWidget:
-    collapsible_widget = CollapsibleWidget(
-        widget_title, expanded_icon="▲", collapsed_icon="▼"
-    )
+    collapsible_widget = CollapsibleWidget(WIDGET_TITLE)
     return collapsible_widget
 
 
@@ -26,7 +24,7 @@ def collapsible_widget_container() -> CollapsibleWidgetContainer:
 def test_collapsible_widget_empty(qtbot, collapsible_widget):
     qtbot.addWidget(collapsible_widget)
 
-    assert collapsible_widget.text() == widget_title
+    assert collapsible_widget.text() == WIDGET_TITLE
     assert not collapsible_widget.isExpanded()
     assert collapsible_widget.content().layout().count() == 0
 
@@ -55,10 +53,18 @@ def test_collapsible_widget_click_once(qtbot, collapsible_widget):
 def test_collapsible_widget_click_multiple(
     qtbot, collapsible_widget, num_clicks
 ):
+    """
+    Test that the toggled_signal_with_self signal is emitted the correct
+    number of times and with the correct parameters when clicked multiple
+    times.
+    """
     qtbot.addWidget(collapsible_widget)
 
     current_state = collapsible_widget.isExpanded()
 
+    # Function to check the signal is emitted with the correct parameters
+    # The signaller should be the collapsible widget and the state should
+    # match the state tracked manually in current_state
     def check_signal_valid(signaller, state):
         return signaller == collapsible_widget and state == current_state
 
@@ -68,6 +74,7 @@ def check_signal_valid(signaller, state):
         timeout=1000,
     ):
         for _ in range(num_clicks + 1):
+            # Invert the current state to simulate a toggle manually
             current_state = not current_state
             collapsible_widget._toggle_btn.click()
 
@@ -121,7 +128,7 @@ def test_collapsible_widget_container_add_remove_widgets(
     assert len(collapsible_widget_container.collapsible_widgets) == 0
 
 
-def test_collapsible_widget_container_add_diff_widgets(
+def test_collapsible_widget_container_add_remove_diff_widgets(
     qtbot, collapsible_widget, collapsible_widget_container
 ):
     qtbot.addWidget(collapsible_widget_container)
@@ -155,17 +162,46 @@ def test_collapsible_widget_container_update_drawers(
     num_other_widgets,
     index_expanded,
 ):
+    """
+    Test that the collapsible widgets in the container are expanded/collapsed
+    correctly when the _update_drawers function is called.
+
+    A container is created with num_collapsible_widgets collapsible widgets
+    and num_other_widgets other widgets. The index_expanded collapsible widget
+    is expanded and the state of each widget is checked. index_expanded is
+    then incremented (modulo num_collapsible_widgets) to simulate toggling
+    each collapsible widget in sequence. Checks the state of each widget after
+    every iteration.
+    """
     qtbot.addWidget(collapsible_widget_container)
     collapsible_widgets = []
     non_collapsible_widgets = []
 
-    for _ in range(num_collapsible_widgets):
-        collapsible_widgets.append(CollapsibleWidget(widget_title))
-        collapsible_widget_container.add_widget(collapsible_widgets[-1])
-
-    for _ in range(num_other_widgets):
-        non_collapsible_widgets.append(QLabel("test"))
-        collapsible_widget_container.add_widget(non_collapsible_widgets[-1])
+    # Add collapsible widgets and other widgets to the container alternating
+    # until the correct number of each type of widget has been added
+    for i in range(num_collapsible_widgets + num_other_widgets):
+        if i % 2 == 0:
+            if len(collapsible_widgets) == num_collapsible_widgets:
+                non_collapsible_widgets.append(QLabel("test"))
+                collapsible_widget_container.add_widget(
+                    non_collapsible_widgets[-1]
+                )
+            else:
+                collapsible_widgets.append(CollapsibleWidget(WIDGET_TITLE))
+                collapsible_widget_container.add_widget(
+                    collapsible_widgets[-1]
+                )
+        else:
+            if len(non_collapsible_widgets) == num_other_widgets:
+                collapsible_widgets.append(CollapsibleWidget(WIDGET_TITLE))
+                collapsible_widget_container.add_widget(
+                    collapsible_widgets[-1]
+                )
+            else:
+                non_collapsible_widgets.append(QLabel("test"))
+                collapsible_widget_container.add_widget(
+                    non_collapsible_widgets[-1]
+                )
 
     for _ in range(num_collapsible_widgets):
         collapsible_widgets[index_expanded]._toggle_btn.click()