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

Reorganize trial conditions and display settings #285

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ZoeJacky
Copy link
Collaborator

No description provided.

@ZoeJacky ZoeJacky requested a review from lkeegan February 18, 2025 14:12
Copy link
Member

@lkeegan lkeegan left a comment

Choose a reason for hiding this comment

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

thanks @ZoeJacky looks good, I added some comments about the bits we discussed

if not dialog.OK:
return None
dialog = TreeDialog(trial)
if dialog.exec_() == QDialog.Accepted:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if dialog.exec_() == QDialog.Accepted:
if dialog.exec() == QDialog.Accepted:

@@ -3,9 +3,11 @@
from typing import Callable

from psychopy.visual.window import Window
from PyQt5.QtGui import QFont
Copy link
Member

Choose a reason for hiding this comment

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

If you use qtpy here instead of PyQt5 then it also works with PyQt6, pyside, etc

Comment on lines +48 to +52
child_item = QtWidgets.QTreeWidgetItem(parent_item)
checkbox = QtWidgets.QCheckBox(f"{labels[key]}")
tree_widget.setItemWidget(child_item, 0, checkbox)
checkbox.clicked.connect(self._update_value_callback(key))
self._widgets[key] = checkbox
Copy link
Member

Choose a reason for hiding this comment

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

this code is copy&pasted below so maybe it could be a function?

tree_widget.setItemWidget(item, 0, checkbox)
checkbox.clicked.connect(self._update_value_callback(key))
self._widgets[key] = checkbox

self.setLayout(outer_layout)
Copy link
Member

Choose a reason for hiding this comment

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

see https://github.com/ssciwr/vstt/blob/main/tests/test_display_widget.py for the existing tests of this widget as an example of how to test these gui elements

Note these tests still pass even though you changed this widget, as they iterate over the checkboxes in the display so are not affected by the checkboxes now being inside a treewidget, but maybe you could add something there to check that the tree widget has the expected groups?

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.

2 participants