feat: add JSON export functionality for snapshot results#233
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #233 +/- ##
==========================================
- Coverage 71.36% 70.96% -0.40%
==========================================
Files 63 64 +1
Lines 4767 4881 +114
==========================================
+ Hits 3402 3464 +62
- Misses 1365 1417 +52 ☔ View full report in Codecov by Sentry. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a first-class “snapshot results” JSON export pathway (CLI + TUI) so snapshot picks and quality metrics can be consumed by downstream tooling, alongside documentation and test coverage updates.
Changes:
- Implement
dump_snapshot_results()plus Pydantic result envelope models for structured JSON export (including per-seismogram quality metrics). - Add
aimbat snapshot resultsCLI command and TUI “Save results to JSON” action (plusaimbat tuicommand refactor andaimbat-tuientry point update). - Update docs navigation and usage guides; add integration/functional tests for the new export behaviour.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zensical.toml | Adds “Exporting Results” to docs navigation and renames/reorders sections. |
| uv.lock | Dependency lock updates (Cyclopts/Griffe/Zensical/Pysmo changes; urllib3 added). |
| tests/integration/core/test_snapshots.py | Integration tests for dump_snapshot_results() output shape and aliasing. |
| tests/functional/test_cli_snapshots.py | Functional CLI tests for snapshot results stdout and --output file writing. |
| tests/conftest.py | Extends CLI test harness to accept token lists (for path-safe invocation). |
| src/aimbat/plot/_iccs.py | Adds update_bandpass() plotting/interactive parameter update wrapper. |
| src/aimbat/models/_readers.py | Introduces SnapshotResults / SnapshotSeismogramResult models for JSON export. |
| src/aimbat/core/_snapshot.py | Implements dump_snapshot_results() with eager-loading and envelope assembly. |
| src/aimbat/app.py | Registers new tool and tui command groups on the root CLI. |
| src/aimbat/_tui/modals.py | Adds “Save results” snapshot action and “Bandpass” tool option in TUI menus. |
| src/aimbat/_tui/help/tab-snapshots.md | Documents the new “Save results to JSON” snapshot action. |
| src/aimbat/_tui/app.py | Implements JSON file-save flow for snapshot results; adds bandpass tool; adds more logging. |
| src/aimbat/_cli/tui.py | New CLI module to launch the TUI via aimbat tui / aimbat-tui. |
| src/aimbat/_cli/tool.py | Renames interactive command group to tool and adds tool bandpass. |
| src/aimbat/_cli/snapshot.py | Adds snapshot results subcommand with --output and --alias support. |
| src/aimbat/_cli/init.py | Exposes tool and tui apps from the CLI package. |
| pyproject.toml | Updates aimbat-tui console script to point at the new Cyclopts app. |
| docs/usage/snapshots.md | Adds “Saving results” section describing the new export command and TUI action. |
| docs/usage/results.md | New page documenting results JSON schema and usage examples. |
| docs/usage/quality.md | Removes outdated reference to the old interactive CC command naming. |
| docs/usage/mccc.md | Renames page/title and adds results-export guidance post-MCCC. |
| docs/usage/index.md | Updates quickstart + TUI invocation docs; documents snapshot action menu additions. |
| docs/usage/alignment.md | Updates interactive-tool docs for renamed tool commands and new bandpass tool. |
| docs/first-steps/workflow.md | Refreshes workflow narrative to emphasise snapshots and export-at-any-stage. |
| README.md | Updates v2 feature bullets to highlight snapshots + structured JSON export. |
| CHANGELOG.md | Adds a linked issue reference for quality stats refactor entry. |
Comment on lines
94
to
96
| - Quality stats integrated into read models and tui | ||
| - Quality stats integrated into read models and tui ([#232](https://github.com/pysmo/aimbat/issues/232)) | ||
|
|
There was a problem hiding this comment.
The changelog lists the same refactoring item twice (once without an issue link, then again with the link). Please remove the duplicate or consolidate into a single bullet so the release notes don’t repeat the same change.
Suggested change
| - Quality stats integrated into read models and tui | |
| - Quality stats integrated into read models and tui ([#232](https://github.com/pysmo/aimbat/issues/232)) | |
| - Quality stats integrated into read models and tui ([#232](https://github.com/pysmo/aimbat/issues/232)) |
Comment on lines
+119
to
+131
| aimbat tool phase <ID> # adjust t1 by clicking on the stack | ||
| aimbat tool window <ID> # set window_pre / window_post by clicking | ||
| aimbat tool cc <ID> # set min_ccnorm by scrolling the matrix image | ||
| aimbat tool bandpass <ID> # adjust bandpass filter settings interactively | ||
| ``` | ||
|
|
||
| === "Shell" | ||
|
|
||
| ```bash | ||
| pick phase # adjust t1 by clicking on the stack | ||
| pick window # set window_pre / window_post by clicking | ||
| pick ccnorm # set min_ccnorm by scrolling the matrix image | ||
| tool phase # adjust t1 by clicking on the stack | ||
| tool window # set window_pre / window_post by clicking | ||
| tool cc # set min_ccnorm by scrolling the matrix image | ||
| tool bandpass # adjust bandpass filter settings interactively |
There was a problem hiding this comment.
These command comments still refer to min_ccnorm, but the surrounding text and CLI naming are now min_cc / aimbat tool cc. Please update the comments to match the actual parameter name to avoid confusing users.
Comment on lines
+1005
to
+1012
| logger.info(f"User selected event {item_id[:8]}.") | ||
| self._current_event_id = uuid.UUID(item_id) | ||
| self._create_iccs() | ||
| self.refresh_all() | ||
| self.notify("Event selected", timeout=2) | ||
|
|
||
| def _toggle_event_completed(self, item_id: str) -> None: | ||
| logger.info(f"User toggled completed flag for event {item_id[:8]}.") |
There was a problem hiding this comment.
These user-action logs are emitted at INFO for very frequent interactions (e.g. selecting/toggling/resetting). Because logging defaults to INFO and writes to a file, this can create a lot of noise and rapid log growth during normal TUI use. Consider downgrading high-frequency action logs to DEBUG and reserving INFO for lifecycle events (startup, project created, alignment completed, etc.).
Suggested change
| logger.info(f"User selected event {item_id[:8]}.") | |
| self._current_event_id = uuid.UUID(item_id) | |
| self._create_iccs() | |
| self.refresh_all() | |
| self.notify("Event selected", timeout=2) | |
| def _toggle_event_completed(self, item_id: str) -> None: | |
| logger.info(f"User toggled completed flag for event {item_id[:8]}.") | |
| logger.debug(f"User selected event {item_id[:8]}.") | |
| self._current_event_id = uuid.UUID(item_id) | |
| self._create_iccs() | |
| self.refresh_all() | |
| self.notify("Event selected", timeout=2) | |
| def _toggle_event_completed(self, item_id: str) -> None: | |
| logger.debug(f"User toggled completed flag for event {item_id[:8]}.") |
Comment on lines
452
to
456
| logger.error(f"ICCS worker: unexpected error during creation: {exc}") | ||
| self.call_from_thread( | ||
| self.notify, f"ICCS init failed: {exc}", severity="error" | ||
| ) | ||
| self.call_from_thread(setattr, self, "_iccs_creating", False) |
There was a problem hiding this comment.
This unexpected-exception log message drops the traceback, which makes diagnosing background-worker failures much harder. Consider logging the exception with a traceback (e.g. Loguru’s exception logging facilities) so the full stack trace is captured in aimbat.log.
- Updated the snapshot help documentation to include a new option for exporting results to JSON. - Added a new action for saving results to JSON in the modal actions. - Refactored the app command structure to include a new command for the terminal user interface. - Implemented a function to dump snapshot results, including per-seismogram quality metrics. - Created models for snapshot results and seismogram results to facilitate structured JSON output. - Enhanced the ICCS plotting module to support updating bandpass filter parameters. - Added functional and integration tests for the new snapshot results CLI command, ensuring expected output and file writing capabilities. - Updated the navigation structure in the documentation to include a new section for exporting results.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📚 Documentation preview 📚: https://aimbat--233.org.readthedocs.build/en/233/