Add annotation tools and refactor canvas resizing#2
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive annotation tools for medical ultrasound images and refactors the canvas resizing logic to be more dynamic and flexible. The changes enable users to annotate DICOM images with polygon segmentation masks and classification labels (grading scores and DVT status), with support for both LabelMe and Darwin V7 JSON export formats.
Changes:
- Adds a complete annotation system with polygon drawing tools for veins, arteries, clots, and other anatomical structures across multiple frames
- Refactors image display from tk.Label to tk.Canvas to support interactive polygon drawing and overlay annotations
- Implements dynamic canvas resizing that adapts to window size changes while maintaining aspect ratio and properly scaling annotation coordinates
- Adds import/export functionality for annotations in two formats (LabelMe and Darwin V7 JSON v2.0)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| draw.line((center_x - X_size, center_y - X_size, center_x + X_size, center_y + X_size), fill="red", width=5) | ||
| draw.line((center_x - X_size, center_y + X_size, center_x + X_size, center_y - X_size), fill="red", width=5) |
There was a problem hiding this comment.
In the exception handler, draw is used but not defined in this scope when an exception occurs in the try block. If the ImageDraw.Draw(img) call in line 2691 fails or if any line within the try block before drawing fails, the variable draw won't exist, causing a NameError when attempting to use draw.line(). The draw object should be created outside the try block or recreated in the exception handler.
| try: | ||
| norm_ann = os.path.normpath(ann_file_path) | ||
| norm_out = os.path.normpath(output_directory2) | ||
| if not norm_ann.startswith(norm_out + os.sep): |
There was a problem hiding this comment.
Path comparison logic may fail on Windows when paths are on different drives or when one path is at the root. The check norm_ann.startswith(norm_out + os.sep) will incorrectly exclude a file if norm_out is exactly the parent directory without a trailing separator (e.g., if norm_ann is "C:\out\file.dcm" and norm_out is "C:\out", the check becomes "C:\out\file.dcm".startswith("C:\out") which works, but if norm_out already has a trailing separator or is a root directory, edge cases may occur). A more robust approach would be to use os.path.commonpath or ensure both paths are resolved and compared properly.
| if not norm_ann.startswith(norm_out + os.sep): | |
| try: | |
| # Use commonpath to robustly check that norm_ann is under norm_out | |
| if os.path.commonpath([norm_ann, norm_out]) != norm_out: | |
| continue | |
| except ValueError: | |
| # Different drives or otherwise incomparable paths; skip |
| else: | ||
| json_path = os.path.splitext(ann_file_path)[0] + ".json" | ||
| if not os.path.isfile(json_path): | ||
| export_annotations_json(ann_file_path, json_path) |
There was a problem hiding this comment.
Missing required positional arguments in function call. Similar to line 3172, export_annotations_json expects 4 parameters: file_path, output_json_path, image_width, image_height. This call only provides 2 parameters. The image_width and image_height parameters are missing, which will cause a TypeError. These parameters should be provided (can use 0 if dimensions are unknown).
| export_annotations_json(ann_file_path, json_path) | |
| export_annotations_json(ann_file_path, json_path, 0, 0) |
|
|
||
| # Tab 2: Annotations (new) | ||
| annotations_tab_frame = tk.Frame(annotation_notebook) | ||
| annotations_tab_frame.grid_columnconfigure(0, weight=1) |
There was a problem hiding this comment.
The annotations_tab_frame is missing row weight configuration. Row 2 contains the annotation list frame that has sticky="nsew", which means it should expand vertically. However, without setting annotations_tab_frame.grid_rowconfigure(2, weight=1), the annotation list won't expand properly to fill available vertical space. This will cause poor UI behavior when the window is resized.
| annotations_tab_frame.grid_columnconfigure(0, weight=1) | |
| annotations_tab_frame.grid_columnconfigure(0, weight=1) | |
| annotations_tab_frame.grid_rowconfigure(2, weight=1) |
| max_height = max(_pfh - 4, 271) if _pfh > 10 else 271 | ||
|
|
||
| # Scale to fit — works both up (enlarge) and down (shrink) | ||
| scale = min(max_width / orig_width, max_height / orig_height) |
There was a problem hiding this comment.
Potential division by zero when calculating scale factor. If both orig_width and orig_height are zero (which shouldn't normally happen but isn't impossible with malformed data), or if max_width or max_height are zero, the scale calculation min(max_width / orig_width, max_height / orig_height) will raise a ZeroDivisionError. Although there are checks to ensure orig_width/height are non-zero for annotation scale factors, the main scale calculation lacks protection. Add validation to ensure all dimensions are positive before calculating scale.
| scale = min(max_width / orig_width, max_height / orig_height) | |
| if orig_width > 0 and orig_height > 0 and max_width > 0 and max_height > 0: | |
| scale = min(max_width / orig_width, max_height / orig_height) | |
| else: | |
| # Fallback to no scaling if any dimension is invalid to avoid division by zero | |
| scale = 1.0 |
| max_height = max(_pfh - 4, 271) if _pfh > 10 else 271 | ||
|
|
||
| # Scale to fit — works both up (enlarge) and down (shrink) | ||
| scale = min(max_width / orig_width, max_height / orig_height) |
There was a problem hiding this comment.
Potential division by zero when calculating scale factor. Similar to the issue at line 2761, if orig_width or orig_height are zero, the scale calculation will raise a ZeroDivisionError. Add validation to ensure dimensions are positive before calculating scale.
| scale = min(max_width / orig_width, max_height / orig_height) | |
| if orig_width > 0 and orig_height > 0: | |
| scale = min(max_width / orig_width, max_height / orig_height) | |
| else: | |
| # Fallback scale to avoid division by zero for invalid image dimensions | |
| scale = 1.0 |
| data = get_annotation_data(file_path) | ||
| frame_key = str(current_frame_index) | ||
| if frame_key in data["frames"]: | ||
| del data["frames"][frame_key] | ||
| draw_annotations_on_canvas(img_label, file_path, current_frame_index, annotation_scale_x, annotation_scale_y) | ||
| update_annotation_list() | ||
| console_message(f"Cleared annotations for frame {current_frame_index}", level="info") | ||
|
|
||
| def clear_all_annotations(): | ||
| data = get_annotation_data(file_path) | ||
| data["frames"] = {} | ||
| draw_annotations_on_canvas(img_label, file_path, current_frame_index, annotation_scale_x, annotation_scale_y) |
There was a problem hiding this comment.
Potential reference to undefined variables in annotation callback functions. The functions clear_frame_annotations() (line 1212) and clear_all_annotations() (line 1222) reference img_label, annotation_scale_x, annotation_scale_y, and current_frame_index. These functions are defined early in preview_file() (around line 1212), but img_label is only created much later (line 2148) inside an if 'PixelData' in ds: block. If the DICOM file doesn't contain PixelData, or if there's an error before img_label is created, clicking the "Clear Frame" or "Clear All" buttons will cause a NameError. Consider checking if these variables exist before using them, or ensure the buttons are disabled when img_label doesn't exist.
| data = get_annotation_data(file_path) | |
| frame_key = str(current_frame_index) | |
| if frame_key in data["frames"]: | |
| del data["frames"][frame_key] | |
| draw_annotations_on_canvas(img_label, file_path, current_frame_index, annotation_scale_x, annotation_scale_y) | |
| update_annotation_list() | |
| console_message(f"Cleared annotations for frame {current_frame_index}", level="info") | |
| def clear_all_annotations(): | |
| data = get_annotation_data(file_path) | |
| data["frames"] = {} | |
| draw_annotations_on_canvas(img_label, file_path, current_frame_index, annotation_scale_x, annotation_scale_y) | |
| # Ensure required GUI elements and indices exist before proceeding | |
| try: | |
| _img_label = img_label | |
| _sx = annotation_scale_x | |
| _sy = annotation_scale_y | |
| _frame_index = current_frame_index | |
| except NameError: | |
| console_message("No image available; cannot clear frame annotations.", level="warning") | |
| return | |
| data = get_annotation_data(file_path) | |
| frame_key = str(_frame_index) | |
| if frame_key in data["frames"]: | |
| del data["frames"][frame_key] | |
| draw_annotations_on_canvas(_img_label, file_path, _frame_index, _sx, _sy) | |
| update_annotation_list() | |
| console_message(f"Cleared annotations for frame {_frame_index}", level="info") | |
| def clear_all_annotations(): | |
| # Ensure required GUI elements and indices exist before proceeding | |
| try: | |
| _img_label = img_label | |
| _sx = annotation_scale_x | |
| _sy = annotation_scale_y | |
| _frame_index = current_frame_index | |
| except NameError: | |
| console_message("No image available; cannot clear all annotations.", level="warning") | |
| return | |
| data = get_annotation_data(file_path) | |
| data["frames"] = {} | |
| draw_annotations_on_canvas(_img_label, file_path, _frame_index, _sx, _sy) |
| except Exception: | ||
| _fc = 1 | ||
| export_annotations_darwin_json(ann_file_path, darwin_path, frame_count=_fc) |
There was a problem hiding this comment.
Missing required positional arguments in function call. The function export_annotations_darwin_json expects 5 parameters: file_path, output_json_path, image_width, image_height, frame_count. However, this call only provides 3 parameters using named argument frame_count=_fc. The image_width and image_height parameters are missing, which will cause this call to fail with a TypeError. These parameters should be provided (can use 0 if dimensions are unknown).
| except Exception: | |
| _fc = 1 | |
| export_annotations_darwin_json(ann_file_path, darwin_path, frame_count=_fc) | |
| _iw = int(getattr(_ds, 'Columns', 0) or 0) | |
| _ih = int(getattr(_ds, 'Rows', 0) or 0) | |
| except Exception: | |
| _fc = 1 | |
| _iw = 0 | |
| _ih = 0 | |
| export_annotations_darwin_json(ann_file_path, darwin_path, _iw, _ih, frame_count=_fc) |
| try: | ||
| #σχεδίαση του crop area στην εικόνα | ||
| draw = ImageDraw.Draw(img) | ||
| rect_coords = (int(crop_x_start), int(crop_y_start), int(crop_x_end), int(crop_y_end)) | ||
| outline_color = "green" if applied_value==1 else "red"#is_applied or | ||
| draw.rectangle(rect_coords, outline=outline_color, width=5) | ||
| crop_values_apply_btn.config(state="enable") | ||
| except Exception as e: | ||
| #except ValueError as e:#παιζει κ αυτό | ||
| width, height = img.size | ||
| center_x, center_y = width // 2, height // 2 | ||
| X_size = 50 | ||
| draw.line((center_x - X_size, center_y - X_size, center_x + X_size, center_y + X_size), fill="red", width=5) | ||
| draw.line((center_x - X_size, center_y + X_size, center_x + X_size, center_y - X_size), fill="red", width=5) | ||
| crop_values_apply_btn.config(state="disabled") |
There was a problem hiding this comment.
Crop area rectangle is drawn on the original image before resizing, but the image is then resized for display. This means the crop area overlay (green/red rectangle) will be scaled along with the image, which could result in incorrect visual feedback. The crop area rectangle should be drawn on the canvas after the image is placed, using scaled coordinates, to ensure it accurately represents the crop boundaries at the display size. Additionally, the error handling that draws an X also suffers from the same issue - it's drawn on the original image before resize.
Add annotation tools and refactor canvas resizing