Skip to content

Commit 5b049b8

Browse files
cofadeclaude
andcommitted
Five quick-win fixes: #30, #44, #33, #60, #56
- #30: save trained YOLO model with model.save() not model.export() - #44: open all text files (YAML/JSON/TXT, incl. .iap project) as UTF-8 to prevent the Windows cp1252 'charmap' crash - #33: edit the smallest containing polygon so nested annotations are reachable (reuses shoelace calculate_area) - #60: sort the image list alphabetically via a signals-blocked model+view rebuild (not setSortingEnabled), preserving the all_images[i] <-> image_list.item(i) invariant - #56: add imagecodecs dependency and catch the LZW/compressed-TIFF ValueError with an actionable dialog instead of crashing Stacked on the #27/#57 branch (PR #71) because #60/#56 build on the image-list filter work. 157 tests pass; ruff F,E9 clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 99e3069 commit 5b049b8

20 files changed

Lines changed: 497 additions & 80 deletions

docs/05_building_block_view.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ the controller graph.
209209
| Controller | Responsibility |
210210
|------------|----------------|
211211
| `ProjectController` | `.iap` save/load, auto-save, backup/restore, missing-image prompts, window-title sync. Owns the `is_loading_project` autosave guard (load/save round-trip safety, v0.8.12). |
212-
| `ImageController` | Open / load / switch images and slices. TIFF + CZI loaders, the multi-dim `DimensionDialog`, the `[-ndim:]` axis-slice bug fix from the v0.9.0 era. Image-list annotation-status filter (`image_has_annotations`, `apply_image_filter`upstream #27). |
212+
| `ImageController` | Open / load / switch images and slices. TIFF + CZI loaders (with `imagecodecs` codec-error handling — #56), the multi-dim `DimensionDialog`, the `[-ndim:]` axis-slice bug fix from the v0.9.0 era. Image-list annotation-status filter (`image_has_annotations`, `apply_image_filter`#27) and alphabetical sort (`sort_image_list`#60). |
213213
| `AnnotationController` | Annotation CRUD, list sorting, highlight, edit-mode entry/exit, `finish_polygon`, `finish_rectangle`, `replace_annotations` (eraser path). Validates writes before mutating `all_annotations`. |
214214
| `ClassController` | Class add / delete / rename / colour / visibility. `update_slice_list_colors`, `is_class_visible`. |
215215
| `SAMController` | SAM box/points tool lifecycle, debounce timer, `_sam_inference_in_flight` re-entrancy guard (ADR-013), model picker. |

docs/08_crosscutting_concepts.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,37 @@ emitters follow up with `annotationsBatchSaved`
558558
New mutation paths must keep one of those two routes — don't add
559559
bespoke `apply_image_filter()` call sites.
560560

561+
## Image List Sorting — Rebuild, Don't `setSortingEnabled`
562+
563+
The image list is kept alphabetical (upstream issue #60,
564+
`ImageController.sort_image_list`). Two constraints shape the
565+
implementation:
566+
567+
- `currentRowChanged` is wired to `switch_image`, so `setSortingEnabled(True)`
568+
is forbidden — a live re-sort would reorder rows and fire spurious
569+
image switches.
570+
- COCO import (and other positional lookups) assume `all_images[i]`
571+
matches `image_list.item(i)`. So the model and the view are sorted
572+
**together**: `all_images` is sorted, then the list is cleared and
573+
repopulated from it with `blockSignals(True)` around the rebuild, and
574+
the prior (or newly added) selection is restored explicitly. The #27
575+
filter is re-applied at the end of the rebuild.
576+
577+
`update_image_list` routes through `sort_image_list`; `add_images_to_list`
578+
calls it with the first added file selected. It is skipped per-image
579+
during project load (the list is rebuilt once via `update_ui`) to avoid
580+
an O(n²) re-sort.
581+
582+
## TIFF Compression Codecs
583+
584+
Reading an LZW- (or otherwise) compressed TIFF requires the optional
585+
`imagecodecs` package; without it `tifffile` raises `ValueError` mid-read
586+
(upstream issue #56). `imagecodecs` is now a hard dependency, but
587+
`ImageController.add_images_to_list` also catches the codec `ValueError`
588+
(`_is_missing_codec_error`) and shows an actionable "pip install
589+
imagecodecs" dialog, skipping the file instead of crashing or leaving a
590+
half-added entry. Non-codec `ValueError`s still propagate.
591+
561592
## Canvas Decoupling — Signals + CanvasContext
562593

563594
`ImageLabel` (the canvas widget) does **not** hold a reference to

setup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
"numpy>=2.0.0", # pip resolves 2.4+ on Py3.14, 2.2.x on Py3.10 (last 3.10-compatible)
3434
"Pillow>=10.0.0",
3535
"tifffile>=2023.0.0",
36+
"imagecodecs>=2023.1.23", # tifffile needs it for LZW/compressed TIFF (#56)
3637
"czifile>=2019.7.2",
3738
"opencv-python>=4.8.0",
3839
"pyyaml>=6.0.0",

src/digitalsreeni_image_annotator/controllers/annotation_controller.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ def load_annotations(self):
217217
if not file_name:
218218
return
219219

220-
with open(file_name, "r") as f:
220+
with open(file_name, "r", encoding='utf-8') as f:
221221
self.mw.loaded_json = json.load(f)
222222

223223
self.mw.class_list.clear()
@@ -259,9 +259,11 @@ def load_annotations(self):
259259

260260
for img in json_images.values():
261261
updated_all_images.append(img)
262-
self.mw.image_list.addItem(img["file_name"])
263262

264263
self.mw.all_images = updated_all_images
264+
# Rebuild the list in sorted order (issue #60). The reconciliation
265+
# loop above already consumed the pre-sort row/index alignment.
266+
self.mw.update_image_list()
265267

266268
self.mw.all_annotations.clear()
267269
for annotation in self.mw.loaded_json["annotations"]:
@@ -538,7 +540,7 @@ def are_all_polygons_connected(polygons):
538540
msg_box.setText("Do you want to keep the original annotations?")
539541
msg_box.setIcon(QMessageBox.Icon.Question)
540542

541-
keep_button = msg_box.addButton("Keep", QMessageBox.ButtonRole.YesRole)
543+
msg_box.addButton("Keep", QMessageBox.ButtonRole.YesRole)
542544
delete_button = msg_box.addButton("Delete", QMessageBox.ButtonRole.NoRole)
543545
cancel_button = msg_box.addButton("Cancel", QMessageBox.ButtonRole.RejectRole)
544546

src/digitalsreeni_image_annotator/controllers/image_controller.py

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,56 @@ def __init__(self, main_window):
8686
self.mw = main_window
8787

8888
def update_image_list(self):
89+
# Rebuild (and sort) the list, preserving the current selection
90+
# without switching images.
91+
self.sort_image_list()
92+
93+
def sort_image_list(self, select_name=None, do_switch=False):
94+
"""Populate image_list in alphabetical order (upstream issue #60).
95+
96+
Sorts the model (`all_images`) and the view together so the
97+
`all_images[i]` ↔ `image_list.item(i)` positional invariant holds
98+
(relied on by COCO import reconciliation). `setSortingEnabled` is
99+
deliberately NOT used: `currentRowChanged` is wired to
100+
`switch_image`, so a live re-sort would fire spurious image
101+
switches. We rebuild with signals blocked instead, then re-select
102+
explicitly.
103+
104+
select_name: file to select after the rebuild (defaults to the
105+
previously-current item). do_switch: call switch_image once for
106+
the selected item (used when adding new images).
107+
"""
108+
current = None
109+
if self.mw.image_list.currentItem() is not None:
110+
current = self.mw.image_list.currentItem().text()
111+
112+
self.mw.all_images.sort(
113+
key=lambda info: (
114+
info.get("file_name", "").casefold(),
115+
info.get("file_name", ""),
116+
)
117+
)
118+
119+
self.mw.image_list.blockSignals(True)
89120
self.mw.image_list.clear()
90-
for image_info in self.mw.all_images:
91-
self.mw.image_list.addItem(image_info["file_name"])
121+
for info in self.mw.all_images:
122+
self.mw.image_list.addItem(info["file_name"])
123+
self.mw.image_list.blockSignals(False)
124+
92125
self.apply_image_filter()
93126

127+
target = select_name if select_name is not None else current
128+
if target is not None:
129+
items = self.mw.image_list.findItems(
130+
target, Qt.MatchFlag.MatchExactly
131+
)
132+
if items:
133+
self.mw.image_list.blockSignals(True)
134+
self.mw.image_list.setCurrentItem(items[0])
135+
self.mw.image_list.blockSignals(False)
136+
if do_switch:
137+
self.switch_image(items[0])
138+
94139
def image_has_annotations(self, image_info):
95140
"""True if the image (or, for multi-dim images, any of its slices)
96141
has at least one annotation."""
@@ -181,7 +226,7 @@ def open_images(self):
181226
self.add_images_to_list(file_names)
182227

183228
def add_images_to_list(self, file_names):
184-
first_added_item = None
229+
first_added_name = None
185230
for file_name in file_names:
186231
base_name = os.path.basename(file_name)
187232
if base_name not in self.mw.image_paths:
@@ -194,7 +239,25 @@ def add_images_to_list(self, file_names):
194239
}
195240

196241
if file_name.lower().endswith((".tif", ".tiff", ".czi")):
197-
self.load_multi_slice_image(file_name)
242+
try:
243+
self.load_multi_slice_image(file_name)
244+
except ValueError as e:
245+
# LZW/compressed TIFFs need the optional imagecodecs
246+
# package; without it tifffile raises ValueError and
247+
# the app used to crash (#56). Skip the file with an
248+
# actionable message instead of a half-added entry.
249+
if self._is_missing_codec_error(e):
250+
QMessageBox.critical(
251+
self.mw,
252+
"Cannot open TIFF",
253+
f"'{base_name}' uses a compression that requires "
254+
"the 'imagecodecs' package, which is not "
255+
"installed.\n\nInstall it with:\n"
256+
" pip install imagecodecs\n\n"
257+
"then reopen the image.",
258+
)
259+
continue
260+
raise
198261
base_name_without_ext = os.path.splitext(base_name)[0]
199262
if (
200263
base_name_without_ext in self.mw.image_slices
@@ -218,22 +281,34 @@ def add_images_to_list(self, file_names):
218281
image_info["width"] = image.width()
219282

220283
self.mw.all_images.append(image_info)
221-
item = QListWidgetItem(base_name)
222-
self.mw.image_list.addItem(item)
223-
if first_added_item is None:
224-
first_added_item = item
284+
if first_added_name is None:
285+
first_added_name = base_name
225286

226287
self.mw.image_paths[base_name] = file_name
227288

228-
if first_added_item:
229-
self.mw.image_list.setCurrentItem(first_added_item)
230-
self.switch_image(first_added_item)
231-
232-
self.apply_image_filter()
289+
# Rebuild the list in sorted order and select/switch to the first
290+
# newly added image. Skipped during project load (the list is
291+
# rebuilt once via update_ui afterwards, and load picks row 0) to
292+
# avoid an O(n^2) re-sort per image.
293+
if first_added_name is not None and not self.mw.is_loading_project:
294+
self.sort_image_list(select_name=first_added_name, do_switch=True)
295+
else:
296+
self.apply_image_filter()
233297

234298
if not self.mw.is_loading_project:
235299
self.mw.auto_save()
236300

301+
@staticmethod
302+
def _is_missing_codec_error(exc):
303+
"""True if a tifffile read failed because the imagecodecs package
304+
is unavailable for the TIFF's compression — e.g. LZW (#56).
305+
306+
Matches only the reliable 'imagecodecs' token: tifffile names the
307+
package in every such message. A broader 'compression' match would
308+
silently swallow unrelated ValueErrors behind a misleading dialog.
309+
"""
310+
return "imagecodecs" in str(exc).lower()
311+
237312
def update_all_images(self, new_image_info):
238313
for info in new_image_info:
239314
if not any(

src/digitalsreeni_image_annotator/controllers/project_controller.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def open_specific_project(self, project_file):
116116
try:
117117
self.mw.is_loading_project = True
118118

119-
with open(project_file, "r") as f:
119+
with open(project_file, "r", encoding='utf-8') as f:
120120
project_data = json.load(f)
121121

122122
self.mw.clear_all(show_messages=False)
@@ -490,7 +490,7 @@ def save_project(self, show_message=True):
490490
if dino_cfg["phrases"] or dino_cfg["thresholds"]:
491491
project_data["dino_config"] = dino_cfg
492492

493-
with open(self.mw.current_project_file, "w") as f:
493+
with open(self.mw.current_project_file, "w", encoding='utf-8') as f:
494494
json.dump(image_utils.convert_to_serializable(project_data), f, indent=2)
495495

496496
if show_message:

src/digitalsreeni_image_annotator/dialogs/coco_json_combiner.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def combine_json_files(self):
6363

6464
try:
6565
for file_path in self.json_files:
66-
with open(file_path, 'r') as f:
66+
with open(file_path, 'r', encoding='utf-8') as f:
6767
data = json.load(f)
6868

6969
# Combine categories
@@ -98,7 +98,7 @@ def combine_json_files(self):
9898

9999
output_file, _ = QFileDialog.getSaveFileName(self, "Save Combined JSON", "", "JSON Files (*.json)")
100100
if output_file:
101-
with open(output_file, 'w') as f:
101+
with open(output_file, 'w', encoding='utf-8') as f:
102102
json.dump(combined_data, f, indent=2)
103103
QMessageBox.information(self, "Success", f"Combined JSON saved to {output_file}")
104104

src/digitalsreeni_image_annotator/dialogs/dataset_splitter.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ def split_images_only(self):
160160
QMessageBox.information(self, "Success", "Dataset split successfully!")
161161

162162
def split_images_and_annotations(self):
163-
with open(self.json_file, 'r') as f:
163+
with open(self.json_file, 'r', encoding='utf-8') as f:
164164
coco_data = json.load(f)
165165

166166
image_files = [img['file_name'] for img in coco_data['images']]
@@ -246,7 +246,7 @@ def save_coco_annotations(self, data, subset):
246246
subset_dir = os.path.join(self.output_directory, subset)
247247
os.makedirs(subset_dir, exist_ok=True)
248248
output_file = os.path.join(subset_dir, f"{subset}_annotations.json")
249-
with open(output_file, 'w') as f:
249+
with open(output_file, 'w', encoding='utf-8') as f:
250250
json.dump(data, f, indent=2)
251251

252252
def split_yolo_format(self, coco_data, train_images, val_images, test_images):
@@ -289,7 +289,7 @@ def split_yolo_format(self, coco_data, train_images, val_images, test_images):
289289

290290
# Create YOLO format labels
291291
label_file = os.path.join(labels_dir, os.path.splitext(image_file)[0] + ".txt")
292-
with open(label_file, "w") as f:
292+
with open(label_file, "w", encoding='utf-8') as f:
293293
for ann in annotations:
294294
# Convert COCO class id to YOLO class id
295295
yolo_class = categories[ann["category_id"]]
@@ -310,7 +310,7 @@ def split_yolo_format(self, coco_data, train_images, val_images, test_images):
310310
}
311311
yaml_data.update(yaml_paths) # Add only paths for non-empty splits
312312

313-
with open(os.path.join(self.output_directory, 'data.yaml'), 'w') as f:
313+
with open(os.path.join(self.output_directory, 'data.yaml'), 'w', encoding='utf-8') as f:
314314
yaml.dump(yaml_data, f, default_flow_style=False)
315315

316316
QMessageBox.information(self, "Success", "Dataset and YOLO annotations split successfully!")

src/digitalsreeni_image_annotator/dialogs/dicom_converter.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
import numpy as np
44
from datetime import datetime
55
from PyQt6.QtWidgets import (QDialog, QVBoxLayout, QHBoxLayout, QPushButton, QFileDialog,
6-
QLabel, QProgressDialog, QRadioButton, QButtonGroup,
7-
QMessageBox, QApplication, QGroupBox)
6+
QLabel, QProgressDialog, QRadioButton, QMessageBox, QApplication, QGroupBox)
87
from PyQt6.QtCore import Qt
98
import pydicom
109
from pydicom.pixel_data_handlers.util import apply_voi_lut
@@ -230,7 +229,7 @@ def convert_dicom(self):
230229
metadata_file = os.path.join(self.output_directory,
231230
os.path.splitext(os.path.basename(self.input_file))[0] +
232231
"_metadata.json")
233-
with open(metadata_file, 'w') as f:
232+
with open(metadata_file, 'w', encoding='utf-8') as f:
234233
json.dump(series_metadata, f, indent=2)
235234

236235
# Get physical sizes from metadata

src/digitalsreeni_image_annotator/dialogs/dino_merge_dialog.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import json
88
import math
9-
import os
109
import random
1110
import traceback
1211
from collections import defaultdict
@@ -180,7 +179,7 @@ def _run(self):
180179
# Load and validate
181180
records = []
182181
for path in coco_files:
183-
with open(path) as f:
182+
with open(path, encoding='utf-8') as f:
184183
data = json.load(f)
185184
if not data.get("images") or not data.get("annotations"):
186185
self._log_msg(f" [skip] {path.name}: empty.")
@@ -295,9 +294,9 @@ def _build_coco(imgs):
295294
train_data = _build_coco(train_imgs)
296295
val_data = _build_coco(val_imgs)
297296

298-
with open(out_path / "train.json", "w") as f:
297+
with open(out_path / "train.json", "w", encoding='utf-8') as f:
299298
json.dump(train_data, f, indent=2)
300-
with open(out_path / "val.json", "w") as f:
299+
with open(out_path / "val.json", "w", encoding='utf-8') as f:
301300
json.dump(val_data, f, indent=2)
302301

303302
self._log_msg(f"Train images: {len(train_imgs)}, annotations: {len(train_data['annotations'])}")

0 commit comments

Comments
 (0)