From 7bc588e9a16d214c7e55effa559a4908e6217945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= Date: Tue, 6 Oct 2026 11:56:04 +0200 Subject: [PATCH] Manual score modification step --- copienator_gui/app.py | 32 +++++- copienator_gui/scores.py | 135 ++++++++++++++++++++++++ copienator_gui/workflow.py | 11 ++ tests/test_annotation_import_cleanup.py | 94 +++++++++++++++++ tests/test_gui_scores.py | 113 ++++++++++++++++++++ 5 files changed, 383 insertions(+), 2 deletions(-) create mode 100644 copienator_gui/scores.py create mode 100644 tests/test_annotation_import_cleanup.py create mode 100644 tests/test_gui_scores.py diff --git a/copienator_gui/app.py b/copienator_gui/app.py index ce605ae..ff43e82 100644 --- a/copienator_gui/app.py +++ b/copienator_gui/app.py @@ -26,6 +26,7 @@ from copienator.rubrics import present_rubric_labels from .diagnostics import collect_diagnostics from .batch_monitor import BatchMonitor from .giving_names import GivingNamesPanel, find_name_issues +from .scores import ScoreEditorPanel from .notifications import notify_desktop from .syncthing import SyncthingPanel from .manual_resolution import ManualResolutionPanel @@ -279,6 +280,7 @@ class CopienatorApp(tk.Tk): self._rendering = False self.refaire_panel: RefaireSelection | None = None self.giving_names_panel: GivingNamesPanel | None = None + self.score_editor_panel: ScoreEditorPanel | None = None self.complete_giving_names_button: ttk.Button | None = None self.label_groups_button: ttk.Button | None = None self.pending_refaire_commands: list[list[str]] = [] @@ -809,7 +811,7 @@ class CopienatorApp(tk.Tk): if not step: return self._rendering = True - if step.id == "personal_read": + if step.id in {"personal_read", "edit_scores"}: self.command_box.grid_remove() else: self.command_box.grid() @@ -822,6 +824,7 @@ class CopienatorApp(tk.Tk): "plotting", "manual_resolution", "giving_names", + "edit_scores", "statement_persp", } self.console.configure(height=8 if expanded_form else 14) @@ -835,6 +838,7 @@ class CopienatorApp(tk.Tk): self.arg_vars.clear() self.refaire_panel = None self.giving_names_panel = None + self.score_editor_panel = None self.complete_giving_names_button = None self.label_groups_button = None entry = self.state_store.step(step.id) if self.state_store.evaluation else {} @@ -1050,6 +1054,12 @@ class CopienatorApp(tk.Tk): ) row += 1 return row + if step.id == "edit_scores" and self.evaluation: + self.score_editor_panel = ScoreEditorPanel( + self.form, self.evaluation, self._score_changed + ) + self.score_editor_panel.grid(row=row, column=0, columnspan=2, sticky="ew", pady=8) + row += 1 if step.id == "giving_names" and self.evaluation: self.giving_names_panel = GivingNamesPanel( self.form, self.evaluation @@ -1658,7 +1668,9 @@ class CopienatorApp(tk.Tk): if not self.current_step or not self.state_store.evaluation or self.active_step_id: return self._save_current_form() - self._invalidate_after_step(self.current_step, self._current_variant().id) + # Score edits already invalidate downstream steps when Set saves a change. + if self.current_step.id != "edit_scores": + self._invalidate_after_step(self.current_step, self._current_variant().id) self.state_store.update_step(self.current_step.id, status=status) history: dict[str, object] = { "step": self.current_step.id, @@ -1687,6 +1699,20 @@ class CopienatorApp(tk.Tk): self._populate_tree() self.info_var.set("Renommage validé — aucun fichier modifié.") + def _score_changed(self, copy_id: str, label: str, previous: str, current: str) -> None: + step = self.step_by_id["edit_scores"] + self._invalidate_after_step(step, "default") + self.state_store.update_step(step.id, status="detected") + self.state_store.add_history({ + "step": step.id, "copy_id": copy_id, "label": label, + "previous_score": previous, "score": current, "manual": True, + }) + # Refresh statuses without rebuilding the form between score edits. + for item in self.steps: + if self.tree.exists(item.id): + status = self._step_status(item) + self.tree.item(item.id, values=(STATUS_LABELS.get(status, status),)) + def _complete_giving_names(self) -> None: if ( not self.current_step @@ -2091,6 +2117,8 @@ class CopienatorApp(tk.Tk): self.complete_giving_names_button.configure( state="disabled" if running else "normal" ) + if self.score_editor_panel is not None: + self.score_editor_panel.set_busy(running) def _move_selection(self, delta: int) -> None: if not self.current_step: diff --git a/copienator_gui/scores.py b/copienator_gui/scores.py new file mode 100644 index 0000000..5c51c3c --- /dev/null +++ b/copienator_gui/scores.py @@ -0,0 +1,135 @@ +from __future__ import annotations + +import re +import tkinter as tk +from decimal import Decimal, InvalidOperation +from pathlib import Path +from tkinter import messagebox, ttk + +from copienator import EvaluationWorkspace, atomic_update_json, read_json +from copienator.utils import natural_key +from .giving_names import RETURN_FOLDER_PATTERN + + +def score_path_for_id(evaluation: Path, copy_id: str) -> Path: + identifier = copy_id.strip() + if not re.fullmatch(r"[0-9]+", identifier): + raise ValueError("Saisissez un numéro d’id.") + return_dir = EvaluationWorkspace(evaluation).return_dir + matches = [] + if return_dir.is_dir(): + for folder in return_dir.iterdir(): + match = RETURN_FOLDER_PATTERN.fullmatch(folder.name) + if folder.is_dir() and not folder.is_symlink() and match and int(match["copy_id"]) == int(identifier): + matches.append(folder / "score.json") + if not matches: + raise FileNotFoundError(f"Aucun dossier dans A Rendre pour l’id {identifier}.") + if len(matches) > 1: + raise ValueError(f"Plusieurs dossiers dans A Rendre portent l’id {identifier}.") + if not matches[0].is_file(): + raise FileNotFoundError(f"score.json introuvable : {matches[0]}") + return matches[0] + + +def read_scores(path: Path) -> dict: + scores = read_json(path) + if not isinstance(scores, dict): + raise ValueError(f"score.json doit contenir un objet JSON : {path}") + return scores + + +def set_score(path: Path, label: str, value: str) -> str: + try: + number = Decimal(value.strip().replace(",", ".")) + except InvalidOperation: + raise ValueError("Le score doit être un nombre.") from None + if not number.is_finite() or number < 0: + raise ValueError("Le score doit être un nombre positif ou nul.") + previous = "" + + def update(scores): + nonlocal previous + if not isinstance(scores, dict) or label not in scores: + raise ValueError(f"Label absent de score.json : {label}") + previous = str(scores[label]) + scores[label] = str(number) + + # A Rendre/score.json is often a symlink to the annotation score file. + # Resolve it before atomic replacement so the link continues to work. + atomic_update_json(path.resolve(strict=True), update, default_factory=dict) + return previous + + +class ScoreEditorPanel(ttk.Frame): + def __init__(self, parent, evaluation: Path, on_change): + super().__init__(parent) + self.evaluation = evaluation + self.on_change = on_change + self.current_path = None + self.busy = False + self.id_var = tk.StringVar() + self.label_var = tk.StringVar() + self.score_var = tk.StringVar() + self.status_var = tk.StringVar() + self.columnconfigure(1, weight=1) + ttk.Label(self, text="Id de copie").grid(row=0, column=0, sticky="w", padx=(0, 8), pady=4) + self.id_entry = ttk.Entry(self, textvariable=self.id_var) + self.id_entry.grid(row=0, column=1, sticky="ew", pady=4) + ttk.Label(self, text="Label").grid(row=1, column=0, sticky="w", padx=(0, 8), pady=4) + self.label_entry = ttk.Combobox(self, textvariable=self.label_var) + self.label_entry.grid(row=1, column=1, sticky="ew", pady=4) + ttk.Label(self, text="Score").grid(row=2, column=0, sticky="w", padx=(0, 8), pady=4) + self.score_entry = ttk.Entry(self, textvariable=self.score_var) + self.score_entry.grid(row=2, column=1, sticky="ew", pady=4) + self.set_button = ttk.Button(self, text="Set", command=self.save_score) + self.set_button.grid(row=2, column=2, padx=(8, 0), pady=4) + ttk.Label(self, textvariable=self.status_var, wraplength=650).grid( + row=3, column=0, columnspan=3, sticky="w", pady=8 + ) + self.id_var.trace_add("write", self.load_score) + self.label_var.trace_add("write", self.load_score) + self.score_entry.bind("", lambda _event: self.save_score()) + self.load_score() + + def load_score(self, *_args): + self.current_path = None + self.score_var.set("") + self.label_entry.configure(values=()) + try: + path = score_path_for_id(self.evaluation, self.id_var.get()) + scores = read_scores(path) + self.label_entry.configure(values=sorted(scores, key=natural_key)) + label = self.label_var.get().strip() + if not label: + self.status_var.set("Choisissez ou saisissez un label.") + elif label not in scores: + self.status_var.set(f"Label absent de score.json : {label}") + else: + self.current_path = path + self.score_var.set(str(scores[label])) + self.status_var.set(path.parent.name) + except (OSError, ValueError) as exc: + self.status_var.set(str(exc)) + self.set_busy(self.busy) + + def set_busy(self, busy: bool): + self.busy = busy + self.id_entry.configure(state="disabled" if busy else "normal") + self.label_entry.configure(state="disabled" if busy else "normal") + state = "normal" if self.current_path is not None and not busy else "disabled" + self.score_entry.configure(state=state) + self.set_button.configure(state=state) + + def save_score(self): + if self.current_path is None or self.busy: + return + label = self.label_var.get().strip() + try: + previous = set_score(self.current_path, label, self.score_var.get()) + current = str(read_scores(self.current_path)[label]) + self.score_var.set(current) + if previous != current: + self.on_change(self.id_var.get().strip(), label, previous, current) + self.status_var.set(f"Score enregistré : {self.current_path.parent.name} — {label} : {current}") + except (OSError, ValueError) as exc: + messagebox.showerror("Modification du score", str(exc)) diff --git a/copienator_gui/workflow.py b/copienator_gui/workflow.py index a1b06c5..9012bb8 100644 --- a/copienator_gui/workflow.py +++ b/copienator_gui/workflow.py @@ -679,6 +679,17 @@ def build_workflow(show_personal_steps: bool) -> list[StepDefinition]: ), artifacts=("A Rendre",), ), + StepDefinition( + "edit_scores", + "Finalisation", + "Modifier des scores", + "Saisissez l’id de copie, puis choisissez ou saisissez un label. " + "Le score actuel est chargé automatiquement. Modifiez-le puis cliquez sur Set " + "pour l’enregistrer dans score.json. Cliquez sur « Marquer terminée » quand vous avez fini.", + (manual("default"),), + optional=True, + requires=("A Rendre",), + ), StepDefinition( "personal_create", "Étapes personnelles", diff --git a/tests/test_annotation_import_cleanup.py b/tests/test_annotation_import_cleanup.py new file mode 100644 index 0000000..0f2f11a --- /dev/null +++ b/tests/test_annotation_import_cleanup.py @@ -0,0 +1,94 @@ +import tempfile +import unittest +from contextlib import ExitStack +from pathlib import Path +from unittest.mock import patch + +from copienator import CliError, EvaluationWorkspace, ExitCode +from copienator.annotation_data import AnnotationLoadResult +from copienator.commands import annotating as simple +from copienator.commands import annotating_by_label as grouped +from copienator.commands import annotating_with_checks as checks +from copienator.commands import import_annotations + + +class AnnotationImportCleanupTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.workspace = EvaluationWorkspace(self.root / "Exam") + for directory in ("Copies", "Par label"): + (self.workspace.root / directory).mkdir(parents=True) + (self.workspace.root / "labels").write_text("A\n") + (self.workspace.root / "correction.json").write_text("{}") + self.import_dir = self.root / "Annotées" + self.import_dir.mkdir() + self.patch = patch.object(import_annotations, "IMPORT_DIR", self.import_dir) + self.patch.start() + self.addCleanup(self.patch.stop) + + def test_cleanup_empties_folder_without_following_child_symlinks(self): + (self.import_dir / "old.pdf").write_bytes(b"return") + nested = self.import_dir / "nested" + nested.mkdir() + (nested / "old.jpg").write_bytes(b"return") + outside = self.root / "keep" + outside.mkdir() + (outside / "keep.pdf").write_bytes(b"keep") + (self.import_dir / "link").symlink_to(outside, target_is_directory=True) + import_annotations.clear_import_directory(self.workspace) + self.assertTrue(self.import_dir.is_dir()) + self.assertEqual(list(self.import_dir.iterdir()), []) + self.assertTrue((outside / "keep.pdf").is_file()) + + def test_cleanup_handles_unconfigured_or_missing_folder(self): + for configured in (None, "", self.root / "absent"): + with self.subTest(configured=configured), \ + patch.object(import_annotations, "IMPORT_DIR", configured): + import_annotations.clear_import_directory(self.workspace) + self.assertFalse((self.root / "absent").exists()) + + def test_cleanup_rejects_an_ancestor_of_evaluation(self): + with patch.object(import_annotations, "IMPORT_DIR", self.root), \ + self.assertRaises(CliError): + import_annotations.clear_import_directory(self.workspace) + self.assertTrue(self.workspace.root.is_dir()) + + def test_all_modes_clean_only_when_checked_and_successful(self): + for module in (simple, checks, grouped): + for clear_import, warnings in ((False, []), (True, []), (True, ["missing answer"])): + with self.subTest(mode=module.__name__, checked=clear_import, warnings=warnings): + old = self.import_dir / "old.pdf" + old.write_bytes(b"return") + with ExitStack() as patches: + patches.enter_context(patch.object( + module, "load_annotation_data", + return_value=AnnotationLoadResult({"01": {"A": {}}}, warnings), + )) + if module is simple: + patches.enter_context(patch.object( + simple, "process_correction", return_value=ExitCode.SUCCESS + )) + elif module is checks: + patches.enter_context(patch.object( + checks, "_render_student", return_value="success" + )) + else: + patches.enter_context(patch.object( + grouped, "_generate_groups", return_value=(1, 0) + )) + patches.enter_context(patch.object( + grouped, "_load_label_groups", return_value=[["A"]] + )) + targets = (self.workspace.root,) if module is checks else () + status = module.run(self.workspace, *targets, clear_import=clear_import) + self.assertEqual(status, ExitCode.PARTIAL if warnings else ExitCode.SUCCESS) + self.assertEqual(old.exists(), not (clear_import and not warnings)) + + def test_cli_forwards_cleanup_flag_for_each_mode(self): + for module in (simple, checks, grouped): + with self.subTest(mode=module.__name__), \ + patch.object(module, "run", return_value=ExitCode.SUCCESS) as run: + self.assertEqual(module.main([str(self.workspace.root), "--clear-import"]), 0) + self.assertTrue(run.call_args.kwargs["clear_import"]) diff --git a/tests/test_gui_scores.py b/tests/test_gui_scores.py new file mode 100644 index 0000000..c79bbc8 --- /dev/null +++ b/tests/test_gui_scores.py @@ -0,0 +1,113 @@ +import os +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from copienator import read_json +from copienator_gui.app import CopienatorApp +from copienator_gui.scores import score_path_for_id, set_score +from copienator_gui.workflow import build_workflow + + +class ScoreEditorTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.evaluation = Path(temporary.name) + folder = self.evaluation / "A Rendre" / "Student (01)" + folder.mkdir(parents=True) + self.path = folder / "score.json" + self.path.write_text('{"Ex 1": "2", "Ex 2": "4"}') + + def test_step_is_optional_and_follows_return_preparation_in_both_profiles(self): + for personal in (False, True): + steps = build_workflow(personal) + index = next(index for index, step in enumerate(steps) if step.id == "giving_names") + self.assertEqual(steps[index + 1].id, "edit_scores") + self.assertTrue(steps[index + 1].optional) + self.assertTrue(steps[index + 1].is_manual) + + def test_update_preserves_other_labels_and_follows_symlinks(self): + source = self.evaluation / "source-score.json" + self.path.rename(source) + self.path.symlink_to(source) + path = score_path_for_id(self.evaluation, "1") + self.assertEqual(path, score_path_for_id(self.evaluation, "01")) + self.assertEqual(set_score(path, "Ex 1", "3,5"), "2") + self.assertTrue(path.is_symlink()) + self.assertEqual(read_json(source), {"Ex 1": "3.5", "Ex 2": "4"}) + self.assertEqual(read_json(path), read_json(source)) + + def test_invalid_score_or_missing_label_does_not_modify_file(self): + before = self.path.read_bytes() + for value in ("", "abc", "NaN", "Infinity", "-1"): + with self.subTest(value=value), self.assertRaises(ValueError): + set_score(self.path, "Ex 1", value) + self.assertEqual(self.path.read_bytes(), before) + with self.assertRaises(ValueError): + set_score(self.path, "Missing", "3") + self.assertEqual(self.path.read_bytes(), before) + + def test_missing_and_ambiguous_ids_are_rejected(self): + with self.assertRaises(FileNotFoundError): + score_path_for_id(self.evaluation, "2") + with self.assertRaises(ValueError): + score_path_for_id(self.evaluation, "../1") + (self.path.parent.parent / "Other (1)").mkdir() + with self.assertRaises(ValueError): + score_path_for_id(self.evaluation, "1") + + +@unittest.skipUnless(os.environ.get("DISPLAY"), "Tk requires a display") +class ScoreEditorGuiTests(ScoreEditorTests): + def setUp(self): + super().setUp() + self.app = CopienatorApp(Path.cwd(), True, self.evaluation) + self.addCleanup(self.close_app) + self.app.update() + self.app.tree.selection_set("edit_scores") + self.app.update() + self.panel = self.app.score_editor_panel + + def close_app(self): + for callback in self.app.tk.splitlist(self.app.tk.call("after", "info")): + self.app.after_cancel(callback) + self.app.destroy() + + def test_selection_loads_score_and_set_keeps_editor_open(self): + self.assertEqual(str(self.panel.set_button.cget("state")), "disabled") + self.panel.id_var.set("1") + self.assertEqual(tuple(self.panel.label_entry.cget("values")), ("Ex 1", "Ex 2")) + self.panel.label_var.set("Ex 1") + self.assertEqual(self.panel.score_var.get(), "2") + self.app.state_store.update_step("final_score", status="success") + self.panel.score_var.set("3,5") + self.panel.set_button.invoke() + self.app.update() + self.assertIs(self.app.score_editor_panel, self.panel) + self.assertEqual(self.panel.id_var.get(), "1") + self.assertEqual(read_json(self.path), {"Ex 1": "3.5", "Ex 2": "4"}) + self.assertEqual(self.app.state_store.step("final_score")["status"], "stale") + self.panel.label_var.set("Ex 2") + self.assertEqual(self.panel.score_var.get(), "4") + self.panel.id_var.set("2") + self.assertEqual(self.panel.score_var.get(), "") + self.assertEqual(str(self.panel.set_button.cget("state")), "disabled") + + def test_invalid_score_shows_error_and_running_disables_edits(self): + self.panel.id_var.set("01") + self.panel.label_var.set("Ex 1") + self.panel.score_var.set("bad") + with patch("copienator_gui.scores.messagebox.showerror") as error: + self.panel.set_button.invoke() + error.assert_called_once() + self.assertEqual(read_json(self.path)["Ex 1"], "2") + self.panel.set_busy(True) + self.assertEqual(str(self.panel.set_button.cget("state")), "disabled") + + def test_skipping_without_changes_preserves_downstream_status(self): + self.app.state_store.update_step("final_score", status="success") + self.app._skip_step() + self.assertEqual(self.app.state_store.step("edit_scores")["status"], "skipped") + self.assertEqual(self.app.state_store.step("final_score")["status"], "success")