fix(gui): pop up "File already exists" instead of silently bumping the run
The Run guard called set_scan_kind(), which runs update_filename() and skips to the next free run number before the existence check, so the check never fired and the scan started under a new number without a word. Now the check runs first, then a warning box names the existing master file and the run number it moved to, and the run is blocked. - Simple tab "Run rotation" had no guard at all; same guard added - "Taken" also matches <run>_*_master.h5: X-ray Centering writes <run>_raster2d_master.h5, which the exact name missed - Camera context-menu "Evaluate grid" presses the panel button, so it gets the guard and stays inert while the button is disabled - Dead next_free_run_from() removed Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -1007,7 +1007,9 @@ class MainWindow(QMainWindow):
|
||||
|
||||
self.sample_camera.autofocus.connect(self.daq.autofocus)
|
||||
|
||||
self.sample_camera.evaluate_grid.connect(self.raster.run_grid_scan)
|
||||
# Context-menu "Evaluate grid" presses the panel button so it gets the
|
||||
# same file-exists guard, and stays inert while the button is disabled.
|
||||
self.sample_camera.evaluate_grid.connect(self.data_collection.raster.start_button.click)
|
||||
self.data_collection.raster.evaluate_grid.connect(self.raster.run_grid_scan)
|
||||
self.data_collection.raster.evaluate_grid_auto.connect(self.raster.run_grid_scan_auto)
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import glob
|
||||
import os
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
@@ -119,8 +120,13 @@ class FilePathPanel(QWidget):
|
||||
def _exists_for_run(self, expanded_base_with_run: str) -> bool:
|
||||
# expanded_base_with_run is the base without scan-kind transforms yet
|
||||
effective = self._effective_dataset_base(expanded_base_with_run)
|
||||
# Consider master file and directory as taken
|
||||
return os.path.exists(f"{effective}_master.h5") or os.path.exists(effective)
|
||||
# Taken = master file, directory, or any derived dataset of this run:
|
||||
# X-ray centering writes "<run>_raster2d_master.h5", not "<run>_master.h5".
|
||||
return (
|
||||
os.path.exists(f"{effective}_master.h5")
|
||||
or os.path.exists(effective)
|
||||
or bool(glob.glob(f"{effective}_*_master.h5"))
|
||||
)
|
||||
|
||||
def update_filename(self):
|
||||
dir_name = self.directory_edit.text()
|
||||
@@ -158,7 +164,7 @@ class FilePathPanel(QWidget):
|
||||
|
||||
# Preview label shows the effective path (what will be written)
|
||||
effective = self._effective_dataset_base(self._filename)
|
||||
exists = os.path.exists(f"{effective}_master.h5") or os.path.exists(effective)
|
||||
exists = self._exists_for_run(self._filename)
|
||||
self.file_name_label.setText(effective + "_master.h5")
|
||||
# Empty stylesheet = reset to the THEME text color (a hardcoded
|
||||
# "default" black would be invisible on the dark theme).
|
||||
@@ -231,42 +237,26 @@ class FilePathPanel(QWidget):
|
||||
def effective_path_for_base(self, base_with_run: str) -> str:
|
||||
return self._effective_dataset_base(base_with_run)
|
||||
|
||||
def next_free_run_from(self, start_rn: int) -> tuple[int, str]:
|
||||
# Compute next free run number and updated base
|
||||
dir_name = self.directory_edit.text().replace("{prefix}", self.file_prefix_edit.text())
|
||||
base = dir_name if dir_name.endswith("/") or dir_name == "" else dir_name + "/"
|
||||
base += "run" if self.file_prefix_edit.text() == "" else self.file_prefix_edit.text()
|
||||
def file_path_error_box(self, scan_kind: str) -> bool:
|
||||
"""Click-time guard for the Run buttons: True when the run may start.
|
||||
|
||||
rn = start_rn
|
||||
while rn <= self.run_number_edit.maximum():
|
||||
candidate_base = self._expand_macros(base, rn)
|
||||
if not self._exists_for_run(candidate_base):
|
||||
return rn, candidate_base
|
||||
rn += 1
|
||||
return start_rn, self._expand_macros(base, start_rn)
|
||||
|
||||
def file_path_error_box(self, scan_kind: str):
|
||||
self.set_scan_kind(scan_kind)
|
||||
base = self.filename # same base we emit
|
||||
effective = self.effective_path_for_base(base)
|
||||
if os.path.exists(f"{effective}_master.h5") or os.path.exists(effective):
|
||||
reply = QMessageBox.question(
|
||||
The scan kind is set directly, not via set_scan_kind(): that calls
|
||||
update_filename(), which silently skips to the next free run number,
|
||||
so the existence check below could never fire and the user was never
|
||||
told the file was already there.
|
||||
"""
|
||||
self._scan_kind = scan_kind
|
||||
exists = self._exists_for_run(self.filename)
|
||||
path = self.effective_path_for_base(self.filename) + "_master.h5"
|
||||
# Refresh the label for the clicked kind; on a clash this also moves
|
||||
# the run number to the next free one, as every edit already does.
|
||||
self.update_filename()
|
||||
if exists:
|
||||
QMessageBox.warning(
|
||||
self,
|
||||
"File exists",
|
||||
# f"This file already exists:\n{effective}_master.h5\nDo you wish to overwrite?",
|
||||
f"This file already exists:\n{effective}_master.h5\n Updating run number",
|
||||
QMessageBox.StandardButton.Yes | QMessageBox.StandardButton.No,
|
||||
QMessageBox.StandardButton.No,
|
||||
f"File already exists:\n{path}\n\n"
|
||||
f"Run number moved to {self.run_number_edit.value()}.",
|
||||
)
|
||||
if reply == QMessageBox.StandardButton.No:
|
||||
# bump run and update
|
||||
curr = self.run_number_edit.value()
|
||||
new_rn, _ = self.next_free_run_from(curr + 1)
|
||||
self.run_number_edit.setValue(new_rn)
|
||||
self.update_filename()
|
||||
return False
|
||||
else:
|
||||
return False
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
@@ -515,6 +515,13 @@ class SimpleRotationSettingsPanel(QWidget):
|
||||
|
||||
@Slot()
|
||||
def run_measurement(self):
|
||||
# Same file-exists guard as the Rotation tab; this panel is not a
|
||||
# ScanSettingsPanel, so it has no check_before_run() to inherit.
|
||||
file_path_panel = getattr(self.parent(), "file_path_panel", None)
|
||||
if file_path_panel is not None and not file_path_panel.file_path_error_box(
|
||||
scan_kind="rotation"
|
||||
):
|
||||
return
|
||||
# Send exactly what the panel last calculated and displayed, rather
|
||||
# than re-reading the widgets: dtz, exposure and transmission are a
|
||||
# single consistent solution and must not be mixed with a newer entry.
|
||||
|
||||
@@ -485,3 +485,87 @@ def test_transmission_rows_are_per_mode(panel, qapp, diffraction):
|
||||
raster_label = _grid_widget(raster._layout, 2, 0)
|
||||
assert isinstance(raster_label, QLabel)
|
||||
assert raster_label.text() == "Transmission"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# File path panel: the Run buttons must not overwrite an existing dataset
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def file_panel(qapp):
|
||||
from aare.gui.panels.file_path_panel import FilePathPanel
|
||||
|
||||
p = FilePathPanel()
|
||||
p.directory_edit.setText("d")
|
||||
p.file_prefix_edit.setText("x")
|
||||
p.run_number_edit.setValue(1)
|
||||
return p
|
||||
|
||||
|
||||
def _taken(monkeypatch, *suffixes):
|
||||
from aare.gui.panels import file_path_panel
|
||||
|
||||
monkeypatch.setattr(
|
||||
file_path_panel.os.path, "exists", lambda path: any(path.endswith(s) for s in suffixes)
|
||||
)
|
||||
|
||||
|
||||
def test_run_is_blocked_with_a_popup_when_the_file_exists(file_panel, monkeypatch):
|
||||
from PySide6.QtWidgets import QMessageBox
|
||||
|
||||
_taken(monkeypatch, "data/d/x_001_master.h5")
|
||||
boxes = []
|
||||
monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: boxes.append(a))
|
||||
|
||||
assert file_panel.file_path_error_box(scan_kind="rotation") is False
|
||||
assert len(boxes) == 1
|
||||
assert "data/d/x_001_master.h5" in boxes[0][2]
|
||||
# The panel moved on to the next free run, so the next click can go ahead.
|
||||
assert file_panel.run_number_edit.value() == 2
|
||||
assert file_panel.file_path_error_box(scan_kind="rotation") is True
|
||||
assert len(boxes) == 1
|
||||
|
||||
|
||||
def test_run_proceeds_when_the_file_is_free(file_panel, monkeypatch):
|
||||
from PySide6.QtWidgets import QMessageBox
|
||||
|
||||
_taken(monkeypatch)
|
||||
monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: pytest.fail("no popup expected"))
|
||||
|
||||
assert file_panel.file_path_error_box(scan_kind="rotation") is True
|
||||
assert file_panel.run_number_edit.value() == 1
|
||||
|
||||
|
||||
def test_a_derived_dataset_counts_as_taken(file_panel, monkeypatch):
|
||||
# X-ray centering writes <run>_raster2d_master.h5, not <run>_master.h5.
|
||||
from aare.gui.panels import file_path_panel
|
||||
|
||||
_taken(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
file_path_panel.glob,
|
||||
"glob",
|
||||
lambda pat: ["hit"] if pat.endswith("raster/d/x_001_*_master.h5") else [],
|
||||
)
|
||||
monkeypatch.setattr(file_path_panel.QMessageBox, "warning", lambda *a, **k: None)
|
||||
|
||||
assert file_panel.file_path_error_box(scan_kind="raster") is False
|
||||
assert file_panel.run_number_edit.value() == 2
|
||||
|
||||
|
||||
def test_simple_tab_run_is_blocked_by_the_file_guard(qapp):
|
||||
from PySide6.QtWidgets import QWidget
|
||||
|
||||
from aare.gui.panels.smart_rotation_panel import SimpleRotationSettingsPanel
|
||||
|
||||
holder = QWidget()
|
||||
cast(Any, holder).file_path_panel = types.SimpleNamespace(
|
||||
file_path_error_box=lambda scan_kind: False
|
||||
)
|
||||
panel = SimpleRotationSettingsPanel(parent=holder)
|
||||
requests = []
|
||||
panel.rotation_scan.connect(requests.append)
|
||||
|
||||
panel.run_measurement()
|
||||
|
||||
assert requests == []
|
||||
|
||||
Reference in New Issue
Block a user