Add popups confirming saves and calculation start#33
Conversation
|
Fixes #26 |
There was a problem hiding this comment.
Pull request overview
Adds user-facing popups to confirm successful saves and to notify when a notebook calculation begins running, plus small readability/formatting cleanups.
Changes:
- Added new popup helpers for “calculation started” and “files saved”, and wired them into multiple save/run flows.
- Added save success confirmations across close-coupling, time-independent, and time-dependent pages (with optional suppression for nested saves).
- Minor refactors for readability (e.g.,
Path.read_text(), loop variable naming, string formatting).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/astra_gui/utils/popup_module.py |
Introduces started_calculation_popup and save_success_popup helpers. |
src/astra_gui/utils/notebook_module.py |
Shows “calculation started” popup when launching background scripts; minor file-read refactor. |
src/astra_gui/time_independent/ti_notebook_page_module.py |
Shows save-success popup after saving TI scripts. |
src/astra_gui/time_independent/structural.py |
String formatting cleanup for generated command lines. |
src/astra_gui/time_dependent/pulse.py |
Shows save-success popup after writing pulse files. |
src/astra_gui/close_coupling/molecule.py |
Shows save-success popup after saving molecule inputs. |
src/astra_gui/close_coupling/lucia.py |
Adds show_popup flag to save() and shows save-success popup when enabled. |
src/astra_gui/close_coupling/dalton.py |
Shows save-success popup after saving DALTON inputs. |
src/astra_gui/close_coupling/clscplng.py |
Suppresses nested Bsplines save popup and shows a single save-success popup for CLSCPLNG. |
src/astra_gui/close_coupling/bsplines.py |
Adds show_popup flag to save() and conditionally shows save-success popup. |
src/astra_gui/app.py |
Minor variable rename cleanup in notebook iteration. |
.codex/environments/environment.toml |
Adds a Codex environment config to run hatch run all. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.controller.after( | ||
| 0, | ||
| lambda: started_calculation_popup(f'{script_name} started running.'), | ||
| ) |
There was a problem hiding this comment.
started_calculation_popup is scheduled before verifying that the background launch command actually succeeded. If the local nohup subprocess returns a non-zero code or the remote run_remote_command returns exit_code != 0, the user will still see “started running”, which is inaccurate. Consider moving this popup to after the launch succeeds (after checking result.returncode == 0 locally / exit_code == 0 remotely), or changing the message to something like “launch requested” and only showing “started” on success.
| self.save_script(self.SCRIPT_FILE, commands, f'{self.label} calculation', convert_cs_irreps=True) | ||
| save_success_popup(f'{self.label} inputs saved successfully.') |
There was a problem hiding this comment.
save_success_popup is shown unconditionally after save_script(...), but save_script can return early without writing anything (e.g., when idle_processor_popup(...) is declined). This can lead to a false “inputs saved successfully” confirmation. Consider having save_script return a boolean indicating whether it actually saved, or restructuring so the success popup is only shown after the save definitely occurred.
| self.save_script(self.SCRIPT_FILE, commands, f'{self.label} calculation', convert_cs_irreps=True) | |
| save_success_popup(f'{self.label} inputs saved successfully.') | |
| saved = self.save_script( | |
| self.SCRIPT_FILE, | |
| commands, | |
| f'{self.label} calculation', | |
| convert_cs_irreps=True, | |
| ) | |
| if saved: | |
| save_success_popup(f'{self.label} inputs saved successfully.') |
| def started_calculation_popup(message: str) -> None: | ||
| """Display a start message for long-running calculations.""" | ||
| messagebox.showinfo('Calculation Started', message) | ||
|
|
||
|
|
||
| def save_success_popup(message: str) -> None: | ||
| """Display a confirmation message after saving files.""" | ||
| messagebox.showinfo('Files Saved', message) | ||
|
|
There was a problem hiding this comment.
New popup helpers started_calculation_popup and save_success_popup were added here, but the existing popup wrapper test suite (see tests/test_popup_module.py) enumerates the popup functions it expects to forward to the messagebox spy. Please extend/update that test coverage to include these new wrappers so regressions (wrong messagebox method, title, etc.) are caught.
1 participant
Summary
Testing