Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #223 +/- ##
===========================================
- Coverage 94.11% 72.29% -21.82%
===========================================
Files 46 50 +4
Lines 1886 2805 +919
===========================================
+ Hits 1775 2028 +253
- Misses 111 777 +666 ☔ View full report in Codecov by Sentry. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @overload | ||
| def plot_all_seismograms( | ||
| session: Session, return_fig: Literal[True] | ||
| ) -> tuple[plt.Figure, plt.Axes]: ... |
|
|
||
|
|
||
| @overload | ||
| def plot_all_seismograms(session: Session, return_fig: Literal[False]) -> None: ... |
| if not isinstance(self.focused, Tabs): | ||
| try: | ||
| event.pane.query_one(DataTable).focus() | ||
| except Exception: |
| active_id: uuid.UUID | None = None | ||
| try: | ||
| active_id = get_active_event(session).id | ||
| except NoResultFound: |
There was a problem hiding this comment.
Pull request overview
This PR introduces a Textual-based terminal UI for AIMBAT and adds supporting core/CLI/model changes to improve interactive workflows (alignment, parameter editing, snapshots), alongside some test/CI and docs updates.
Changes:
- Add a new Textual TUI (
aimbat._tui) with modal-driven workflows for event switching, alignment, parameter editing, snapshots, and per-row actions. - Extend core + CLI functionality around ICCS lifecycle, snapshot parameter inspection, and seismogram-parameter resets (with accompanying tests).
- Update models and config metadata to support the TUI (parameter titles/units, computed snapshot counts, new settings, docs nav).
Reviewed changes
Copilot reviewed 38 out of 41 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
zensical.toml |
Adds _tui API docs page and reorders API nav entries. |
tox.ini |
Updates tox/uv integration and coverage invocation. |
pyproject.toml |
Adds Textual dependencies and a dedicated aimbat-tui script entry point; removes Qt/pyqtgraph deps. |
src/aimbat/app.py |
Registers the TUI as a CLI subcommand (aimbat tui). |
src/aimbat/_config.py |
Adds configurable TUI theme settings. |
src/aimbat/_tui/__init__.py |
Introduces TUI package. |
src/aimbat/_tui/_widgets.py |
Adds vim-navigation DataTable widget. |
src/aimbat/_tui/modals.py |
Implements modal screens (event switcher, confirm, parameter input, action menus, etc.). |
src/aimbat/_tui/app.py |
Main Textual TUI app: tabbed views, actions, ICCS lifecycle, and DB interactions. |
src/aimbat/_tui/aimbat.tcss |
Styling for the TUI and modal layouts. |
docs/api/aimbat/_tui.md |
Adds mkdocstrings page for the new TUI package. |
src/aimbat/io/_data.py |
Adds DATATYPE_SUFFIXES for data-type-aware file picking. |
src/aimbat/models/_parameters.py |
Adds title= and clarifies units in field descriptions for TUI display. |
src/aimbat/models/_models.py |
Adds snapshot count column-properties; introduces AimbatSeismogram.extra; adjusts typing stubs for computed properties. |
src/aimbat/models/_readers.py |
Moves read/DTO-style SQLModel types into a dedicated module. |
src/aimbat/models/__init__.py |
Re-exports reader models. |
src/aimbat/core/_iccs.py |
Adds ICCS parameter syncing + write-back logic; adapts plotting/picking APIs to optionally return figures. |
src/aimbat/core/_seismogram.py |
Adds seismogram-parameter reset functions; changes plotting API to optionally return figures. |
src/aimbat/core/_snapshot.py |
Improves rollback field coverage and adds snapshot-parameter table printing. |
src/aimbat/core/_active_event.py |
Minor query refactor. |
src/aimbat/core/_event.py |
Minor query refactor; updated imports to new model exports. |
src/aimbat/core/_data.py |
Minor query refactor; improves debug logging when station UUID lookup fails. |
src/aimbat/_cli/common.py |
Removes PlotParameters (Qt/pyqtgraph option). |
src/aimbat/_cli/plot.py |
Removes pyqtgraph plotting path; updates plotting calls to new signatures. |
src/aimbat/_cli/pick.py |
Updates pick commands for new return_fig parameter. |
src/aimbat/_cli/event.py |
Updates timedelta parsing for event-parameter set (bare numbers interpreted as seconds). |
src/aimbat/_cli/seismogram.py |
Adds seismogram parameter reset CLI command. |
src/aimbat/_cli/snapshot.py |
Adds snapshot details command to print event-parameters captured in a snapshot. |
tests/unit/test_app_entrypoint.py |
Adds unit tests for CLI --help and --version behaviour. |
tests/unit/_cli/test_common.py |
Removes tests for deleted PlotParameters. |
tests/integration/tui/test_app.py |
Adds a guard test ensuring event-parameter fields have titles for TUI rendering. |
tests/integration/models/test_operations.py |
Adds integration tests for snapshot count column-properties. |
tests/integration/core/test_snapshots.py |
Adds broader rollback and snapshot-parameter table print tests. |
tests/integration/core/test_seismogram.py |
Adds tests for seismogram-parameter reset functions; updates plotting test to new signature. |
tests/functional/test_cli_parameters.py |
Adds CLI tests for setting timedelta event parameters. |
tests/functional/test_cli_snapshots.py |
Adds CLI tests for the new snapshot details command (including short IDs). |
.github/workflows/run-tests.yml |
Adjusts workflow permissions and Codecov action usage. |
flake.nix |
Removed Nix flake dev shell configuration. |
flake.lock |
Removed Nix flake lockfile. |
.devcontainer/devcontainer.json |
Removed devcontainer configuration. |
| uv sync --active --inexact --python python --no-group docs | ||
| commands = | ||
| py.test --mpl --mypy --cov=aimbat --cov-append --cov-report=term-missing --junitxml=junit.xml | ||
| py.test --mpl --mypy --cov --cov-append --cov-report=term-missing --junitxml=junit.xml |
There was a problem hiding this comment.
In tox, pytest-cov is invoked as --cov with no source path/module. This can change what gets measured (and may include tests or other files) compared to the repo’s usual --cov src/--cov=aimbat usage and [tool.coverage.run].source = ["src"]. Consider specifying the coverage source explicitly (e.g. --cov src or --cov=aimbat) to keep results consistent.
| logger.warning( | ||
| "Returning figure and axes objects instead of showing the plot. This is intended for testing purposes; in normal usage, return_fig should be False." | ||
| ) |
There was a problem hiding this comment.
The logger.warning(...) message is on a single very long line and will violate Ruff’s line-length (88) (E501). Please wrap the message across multiple lines (e.g. implicit string concatenation). This same message appears multiple times in this module.
| data=snapshot.model_dump(mode="json"), | ||
| title=f"Saved event parameters in snapshot:: {uuid_shortener(session, snapshot.snapshot) if short else str(snapshot.snapshot.id)}", | ||
| skip_keys=["id", "snapshot_id", "parameters_id"], |
There was a problem hiding this comment.
The title= f-string is longer than Ruff’s configured line-length (88) and will fail lint (E501). Consider wrapping it across multiple lines (e.g. build the ID string separately or use implicit string concatenation within parentheses).
| extra: dict[Hashable, Any] = Field( | ||
| default_factory=dict, | ||
| sa_column=Column(MutableDict.as_mutable(PickleType)), | ||
| description="Dictionary to store any additional metadata for the seismogram.", | ||
| ) |
There was a problem hiding this comment.
AimbatSeismogram.extra is stored using SQLAlchemy PickleType, which serialises via Python pickle. Unpickling data from a project database can execute arbitrary code if the DB file is ever opened from an untrusted source, and it also makes the schema less portable/inspectable. Prefer a JSON-backed column (e.g. SQLAlchemy JSON + MutableDict, or a TEXT column with JSON encoding) or keep this metadata purely in-memory on the ICCS mini-seismograms rather than persisting it in the database.
| "Number of seismogram parameter snapshots associated with this snapshot that are marked as selected." | ||
|
|
There was a problem hiding this comment.
This standalone string description exceeds the repo Ruff line-length (88) and will fail lint (E501). Please wrap it across multiple lines (implicit string concatenation) or convert it to a comment.
| def plot_stack(iccs: ICCS, context: bool, all: bool, return_fig: bool) -> tuple | None: | ||
| """Plot the ICCS stack. | ||
|
|
||
| Args: | ||
| iccs: ICCS instance. | ||
| context: Whether to use seismograms with extra context. | ||
| all: Whether to plot all seismograms. | ||
| return_fig: If True, return the figure and axes objects instead of showing the plot. | ||
|
|
||
| Returns: | ||
| A tuple of (Figure, Axes) if return_fig is True, otherwise None. | ||
| """ |
There was a problem hiding this comment.
Several exported core functions now require a return_fig argument (plot_stack, plot_iccs_seismograms, update_pick, update_timewindow, update_min_ccnorm). Because aimbat.core re-exports these as part of the public API, making this parameter required is a breaking change. Consider giving return_fig a default of False (and ideally making it keyword-only) to preserve backwards compatibility.
| def plot_all_seismograms( | ||
| session: Session, return_fig: bool | ||
| ) -> tuple[plt.Figure, plt.Axes] | None: | ||
| """Plot all seismograms for a particular event ordered by great circle distance. | ||
|
|
||
| Args: | ||
| use_qt: Plot with pqtgraph instead of pyplot | ||
| """ | ||
| session: Database session. | ||
| return_fig: Whether to return the figure and axes objects instead of showing the plot. | ||
|
|
||
| active_event = get_active_event(session) | ||
| Returns: | ||
| figure and axes objects if return_fig is True, otherwise None. | ||
| """ | ||
|
|
There was a problem hiding this comment.
plot_all_seismograms() now requires return_fig, which is a breaking change for callers importing it from aimbat.core. Consider defaulting return_fig to False (and making it keyword-only) to keep the previous call pattern working.
| bandpass_fmin: float = Field( | ||
| default_factory=lambda: settings.bandpass_fmin, | ||
| ge=0, | ||
| description="Minimum frequency for bandpass filter (ignored if `bandpass_apply` is False).", | ||
| title="Bandpass f min", | ||
| description="Minimum frequency for bandpass filter in Hz (ignored if `bandpass_apply` is False).", | ||
| ) |
There was a problem hiding this comment.
Several Field(description=...) strings here exceed the repo Ruff line-length (88) and will fail lint (E501). Consider splitting the description into multiple adjacent string literals inside parentheses (implicit concatenation) to keep lines within 88 characters.
| """Run MCCC algorithm. | ||
|
|
||
| Args: | ||
| session: SQLModel session. | ||
| session: Database session. | ||
| iccs: ICCS instance. | ||
| all_seismograms: Whether to include all seismograms in the MCCC processing, or just the selected ones. | ||
| """ |
There was a problem hiding this comment.
This docstring parameter description line is longer than Ruff’s configured line-length (88) and will fail lint (E501). Please wrap the text onto additional lines to keep within the limit.
| "Number of seismogram parameter snapshots associated with this snapshot that are marked as flipped." | ||
|
|
There was a problem hiding this comment.
This standalone string description exceeds the repo Ruff line-length (88) and will fail lint (E501). Please wrap it across multiple lines (implicit string concatenation) or convert it to a comment.
- Add Textual-based TUI (_tui/) with tabs for seismograms, parameters, stations, and snapshots - Add _CSS and _Hint StrEnums in modals.py for shared label strings - Replace _PARAM_META with AimbatEventParametersBase.model_fields; add title= and units to field descriptions - Replace _ICCS_ATTRS with hasattr check - Add DATATYPE_SUFFIXES to io/_data.py for data-type-aware file picker - Add sync_iccs_parameters to core/_iccs.py for rollback without re-reading waveform data - Add reset_seismogram_parameters / reset_seismogram_parameters_by_id to core/_seismogram.py; add CLI command and tests - Add seismogram parameter reset to TUI row-action menu - Validate with ICCS before DB write in _apply_parameter; recover ICCS when parameters are fixed - Refresh main app on event switcher close (covers deletions) - Add tests for print_snapshot_parameters_table and snapshot details CLI - Add JSON datasource support and _readers.py - Remove devcontainer and Nix flake files
| self._iccs.seismograms, self._iccs.ccnorms | ||
| ): | ||
| ccnorm_map[iccs_seis.extra["id"]] = float(ccnorm) | ||
| except Exception: |
| cc, | ||
| key=str(seis.id), | ||
| ) | ||
| except (NoResultFound, RuntimeError): |
| label = field_info.title or attr | ||
| desc = field_info.description or "" | ||
| table.add_row(label, display, desc, key=attr) | ||
| except (NoResultFound, RuntimeError): |
| elev, | ||
| key=str(st.id), | ||
| ) | ||
| except (NoResultFound, RuntimeError): |
| flipped_count, | ||
| key=str(snap.id), | ||
| ) | ||
| except (NoResultFound, RuntimeError): |
📚 Documentation preview 📚: https://aimbat--223.org.readthedocs.build/en/223/