Skip to content

Adds test coverage for drag&drop functionality #13

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.

Sign up for GitHub

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

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

Adds test coverage for drag&drop functionality #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f9bdf0
Okay, here's a summary of the changes I've made:
google-labs-jules[bot] May 27, 2025
50fc885
Refactor: Use pytest-qt for SubPlotWidget D&D tests
google-labs-jules[bot] May 27, 2025
42732da
Refactor: Use pytest-qt for SubPlotWidget DND tests
google-labs-jules[bot] Jun 10, 2025
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
91 changes: 51 additions & 40 deletions custom_plot_item.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# -*- coding: utf-8 -*-

from PyQt5.QtCore import Qt, pyqtSlot, QRect, QMimeData, QByteArray
from PyQt5.QtWidgets import QLabel, QMenu, QAction
from PyQt5.QtGui import QPalette, QPixmap, QPainter, QDrag
from PyQt5.QtCore import Qt, pyqtSlot, QRect, QMimeData, QByteArray, QPoint # Added QPoint
from PyQt5.QtWidgets import QLabel, QMenu, QAction, QApplication # QApplication moved here
from PyQt5.QtGui import QPalette, QPixmap, QPainter, QDrag # QDrag was already here
from logging_config import get_logger

try:
from PyQt5.QtGui import QApplication
except ImportError:
from PyQt5.QtWidgets import QApplication
# try: # QApplication is now directly imported from QtWidgets
# from PyQt5.QtGui import QApplication
# except ImportError:
# from PyQt5.QtWidgets import QApplication

import os
import pickle # Add pickle
Expand All @@ -34,7 +34,7 @@ def __init__(self, parent, plot_data_item, source, current_tick):

self.trace = plot_data_item
self._subplot_widget = parent # Store parent reference
self._drag_start_position = None # Initialize drag start position
self.drag_start_position = QPoint() # Initialize drag start position, ensure it's QPoint

self.source = source

Expand Down Expand Up @@ -168,58 +168,69 @@ def mousePressEvent(self, event):
self.remove_item()
event.accept()
elif event.button() == Qt.LeftButton:
self._drag_start_position = event.pos()
self.drag_start_position = event.pos() # Use self.drag_start_position
else:
super().mousePressEvent(event)

def mouseMoveEvent(self, event):
if not (event.buttons() & Qt.LeftButton) or not self._drag_start_position:
if not (event.buttons() & Qt.LeftButton):
# Call super for other mouse move events if necessary, or just return
super().mouseMoveEvent(event)
return
if (event.pos() - self._drag_start_position).manhattanLength() < QApplication.startDragDistance():

# Check if drag_start_position is initialized and valid
if not hasattr(self, 'drag_start_position') or self.drag_start_position is None or self.drag_start_position.isNull():
super().mouseMoveEvent(event)
return

if (event.pos() - self.drag_start_position).manhattanLength() < QApplication.startDragDistance():
super().mouseMoveEvent(event)
return

# Start drag
# logger.info(f"CustomPlotItem '{self.name}': Initiating QDrag.")
drag = QDrag(self)
mime_data = QMimeData()

try:
mime_data.setText(self.trace.name())
except Exception as e_setText:
logger.error(f"MOUSE_MOVE_EVENT: Exception during setText: {e_setText}")
# This is where the pickling error might be if self.trace.name() is complex
# and indirectly tries to pickle self.source. Unlikely but possible.

try:
if self._subplot_widget and self._subplot_widget.objectName():
mime_data.setData("application/x-customplotitem-sourcewidget", QByteArray(self._subplot_widget.objectName().encode()))
else:
logger.warning("MOUSE_MOVE_EVENT: _subplot_widget or its objectName not set.")
mime_data.setData("application/x-customplotitem-sourcewidget", QByteArray())
except Exception as e_setSourceWidget:
logger.exception(f"MOUSE_MOVE_EVENT: Exception during setData for sourcewidget: {e_setSourceWidget}")
# Set text for general purpose (e.g., if dropped on a text editor or for SubPlotWidget's text() check)
mime_data.setText(self.trace.name())
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jun 1, 2025

Choose a reason for hiding this comment

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

medium

The previous version of this code had specific try-except blocks around mime_data.setText() and mime_data.setData() calls, logging errors if they occurred. The current refactoring has removed these.

If self.trace.name() were to return None (which PlotDataItem.name() can, as it returns self.opts.get('name', None)), then mime_data.setText(self.trace.name()) might pass None to Qt, which could lead to unexpected behavior or errors depending on Qt's handling of None for string conversion. Can we consider adding a check for self.trace.name() being None or reintroducing a broader try-except block around the MIME data population to catch potential exceptions and log them, ensuring the drag operation fails gracefully or can be aborted cleanly?

if self.trace.name() is not None:
    mime_data.setText(self.trace.name())
else:
    mime_data.setText("") # Or some default string


# Set specific data for "application/x-customplotitem" format
# This is what SubPlotWidget's dropEvent will primarily check via hasFormat and then use text().
# Encoding the trace name as QByteArray for setData.
mime_data.setData("application/x-customplotitem", self.trace.name().encode())
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jun 1, 2025

Choose a reason for hiding this comment

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

high

The previous version of this code had specific try-except blocks around mime_data.setText() and mime_data.setData() calls, logging errors if they occurred. The current refactoring has removed these.

If self.trace.name() were to return None (which PlotDataItem.name() can, as it returns self.opts.get('name', None)), then self.trace.name().encode() would raise an AttributeError ('NoneType' object has no attribute 'encode'). This could cause an unhandled exception during the drag operation, potentially crashing or destabilizing the drag. Can we consider adding a check for self.trace.name() being None or reintroducing a broader try-except block around the MIME data population to catch potential exceptions and log them, ensuring the drag operation fails gracefully or can be aborted cleanly?

if self.trace.name() is not None:
    mime_data.setData("application/x-customplotitem", self.trace.name().encode())
else:
    mime_data.setData("application/x-customplotitem", QByteArray())


# It seems SubPlotWidget.dropEvent for CustomPlotItem reordering/moving
# also uses e.mimeData().data("application/x-customplotitem-sourcewidget")
# to get the source widget's object name. This should be preserved if still used.
# The existing code already has this:
if self._subplot_widget and self._subplot_widget.objectName():
mime_data.setData("application/x-customplotitem-sourcewidget",
QByteArray(self._subplot_widget.objectName().encode()))
else:
# logger.warning("CustomPlotItem.mouseMoveEvent: _subplot_widget or its objectName not set.")
mime_data.setData("application/x-customplotitem-sourcewidget", QByteArray())


try:
mime_data.setData("application/x-customplotitem", QByteArray()) # The marker
except Exception as e_setCustomPlotItem:
logger.exception(f"MOUSE_MOVE_EVENT: Exception during setData for application/x-customplotitem: {e_setCustomPlotItem}")

drag.setMimeData(mime_data)

# Visual feedback for the drag
try:
pixmap = QPixmap(self.size())
self.render(pixmap)
pixmap = self.grab() # Grab the current appearance of the label
drag.setPixmap(pixmap)
# Set the hot spot to be where the mouse click started within the label
drag.setHotSpot(event.pos() - self.rect().topLeft())
except Exception as e_pixmap:
logger.exception(f"MOUSE_MOVE_EVENT: Exception during pixmap creation/setting: {e_pixmap}")
# If self.render() or self.size() somehow trigger the pickle error via self.source
logger.exception(f"CustomPlotItem.mouseMoveEvent: Exception during pixmap creation/setting: {e_pixmap}")

try:
drag.exec_(Qt.MoveAction)
except Exception as e_drag:
logger.exception(f"MOUSE_MOVE_EVENT: Error during drag.exec_(): {e_drag}")
# logger.info(f"CustomPlotItem '{self.name}': Executing drag.")
drag.exec_(Qt.MoveAction)
# logger.info(f"CustomPlotItem '{self.name}': Drag finished.")

self._drag_start_position = None
# Reset drag_start_position after drag finishes, though it might be good practice
# to reset it in mouseReleaseEvent as well, or if the drag is cancelled.
# For now, this matches the original logic of setting it to None.
self.drag_start_position = QPoint() # Reset to an invalid/default QPoint

def remove_item(self):
self.parent().remove_item(self.trace, self)
Expand Down
Loading