fix(gui): make the file-exists guard literal: popup, run +1, no scan
CI / lint (push) Skipped
CI / test (3.12) (push) Skipped
CI / test (3.13) (push) Skipped
CI / test-with-beamline-plugins (pxi_bec) (push) Skipped
CI / test-with-beamline-plugins (pxii_bec) (push) Skipped
CI / test-with-beamline-plugins (pxiii_bec) (push) Skipped
CI / lint (push) Skipped
CI / test (3.12) (push) Skipped
CI / test (3.13) (push) Skipped
CI / test-with-beamline-plugins (pxi_bec) (push) Skipped
CI / test-with-beamline-plugins (pxii_bec) (push) Skipped
CI / test-with-beamline-plugins (pxiii_bec) (push) Skipped
update_filename() still skipped to the next free run number on every edit and refresh, so the name was always free by the time Run was clicked and the guard never fired. The name is now exactly what the fields say (label turns red on a clash); on Run, an existing target pops "File already exists", bumps the run number by one and does nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -144,23 +144,11 @@ class FilePathPanel(QWidget):
|
||||
|
||||
base += "run" if file_prefix == "" else file_prefix
|
||||
|
||||
# Find next free run number using effective dataset path
|
||||
rn = run_number
|
||||
while True:
|
||||
candidate_base = self._expand_macros(base, rn)
|
||||
if not self._exists_for_run(candidate_base):
|
||||
break
|
||||
rn += 1
|
||||
if rn > self.run_number_edit.maximum():
|
||||
break
|
||||
|
||||
if rn != run_number:
|
||||
self.run_number_edit.blockSignals(True)
|
||||
self.run_number_edit.setValue(rn)
|
||||
self.run_number_edit.blockSignals(False)
|
||||
|
||||
# Literal: the name is exactly what the fields say. No silent skip to
|
||||
# the next free run here, or the click-time guard below never sees a
|
||||
# clash; the label just turns red until the user (or the guard) acts.
|
||||
# Store the GUI’s base (without applying scan-kind transforms) for wiring into requests later
|
||||
self._filename = self._expand_macros(base, rn)
|
||||
self._filename = self._expand_macros(base, run_number)
|
||||
|
||||
# Preview label shows the effective path (what will be written)
|
||||
effective = self._effective_dataset_base(self._filename)
|
||||
@@ -240,23 +228,20 @@ class FilePathPanel(QWidget):
|
||||
def file_path_error_box(self, scan_kind: str) -> bool:
|
||||
"""Click-time guard for the Run buttons: True when the run may start.
|
||||
|
||||
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.
|
||||
On a clash: bump the run number by one, tell the user, refuse the run.
|
||||
The next click checks the new name again, so nothing is ever skipped
|
||||
without the user seeing it.
|
||||
"""
|
||||
self._scan_kind = scan_kind
|
||||
exists = self._exists_for_run(self.filename)
|
||||
self.update_filename() # label/path for the clicked kind (screening vs rotation)
|
||||
if not self._exists_for_run(self.filename):
|
||||
return True
|
||||
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"File already exists:\n{path}\n\n"
|
||||
f"Run number moved to {self.run_number_edit.value()}.",
|
||||
)
|
||||
return False
|
||||
return True
|
||||
self.increment_run_number()
|
||||
QMessageBox.warning(
|
||||
self,
|
||||
"File exists",
|
||||
f"File already exists:\n{path}\n\n"
|
||||
f"Run number increased to {self.run_number_edit.value()}.",
|
||||
)
|
||||
return False
|
||||
|
||||
@@ -518,10 +518,16 @@ def test_run_is_blocked_with_a_popup_when_the_file_exists(file_panel, monkeypatc
|
||||
boxes = []
|
||||
monkeypatch.setattr(QMessageBox, "warning", lambda *a, **k: boxes.append(a))
|
||||
|
||||
# Editing never skips a taken run silently: the name stays literal and
|
||||
# the label only turns red.
|
||||
file_panel.set_scan_kind("rotation")
|
||||
assert file_panel.run_number_edit.value() == 1
|
||||
assert file_panel.file_name_label.styleSheet() != ""
|
||||
|
||||
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.
|
||||
# Run number went up by one, 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
|
||||
|
||||
Reference in New Issue
Block a user