refactor: Use default event concept only for cli commands#227
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| from sqlmodel import Session, select | ||
| from textual.widgets import DataTable, Static, TabbedContent, TabPane | ||
|
|
||
| import aimbat._tui.app |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #227 +/- ##
==========================================
+ Coverage 73.20% 73.63% +0.43%
==========================================
Files 51 51
Lines 3064 3050 -14
==========================================
+ Hits 2243 2246 +3
+ Misses 821 804 -17 ☔ View full report in Codecov by Sentry. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors “default event” handling so that the TUI no longer falls back to (or manipulates) the database default event, aligning interactive workflows around explicit event selection. It also updates the repo’s formatting/linting tooling and includes a few operational/core additions that are adjacent to the workflow changes.
Changes:
- TUI: remove default-event indicators/actions; require explicit event selection and update tests/docs accordingly.
- Tooling: migrate formatting checks from Black to
ruff formatacross CI/tox/Makefile and lockfile. - Core/DB: add
build_iccs_from_snapshot()and adjust SQLite engine pragmas/connection args.
Reviewed changes
Copilot reviewed 81 out of 82 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Removes Black from dev deps and updates lock entries for Ruff-based tooling. |
| tox.ini | Switches lint env to ruff check + ruff format --check. |
| pyproject.toml | Replaces Black dev dependency with Ruff and configures Ruff lint settings. |
| Makefile | Updates format, lint, and format-check targets to use Ruff format/lint. |
| .github/workflows/run-tests.yml | CI now checks ruff format --check instead of Black. |
| CHANGELOG.md | Notes “default event concept only for cli commands”. |
| docs/usage/index.md | Reframes “default event” as CLI-only and updates interface description. |
| docs/usage/tui.md | Updates TUI documentation to reflect event selection without default-event markers/actions. |
| docs/usage/api.md | Updates wording around DB constraints/triggers; expands example narrative. |
| docs/snippets/api_load_project.py | Removes “set default event” step; snapshots all events after import. |
| docs/snippets/api_alignment.py | Uses first event rather than get_default_event() in snippet. |
| tests/conftest.py | Import ordering/formatting changes consistent with Ruff formatting. |
| tests/assets/make_events.py | Import ordering/formatting changes. |
| tests/unit/utils/test_sampledata.py | Import spacing/formatting changes. |
| tests/unit/utils/test_json.py | Import spacing/formatting changes. |
| tests/unit/test_config.py | Import ordering/spacing changes. |
| tests/unit/test_app_entrypoint.py | Import spacing/formatting changes. |
| tests/unit/io/test_sac.py | Import grouping/order changes. |
| tests/unit/io/test_json_sources.py | Import grouping/order changes. |
| tests/unit/_types/test_sqlalchemy.py | Import ordering changes. |
| tests/unit/_types/test_pydantic.py | Import ordering changes. |
| tests/unit/_cli/test_common.py | Import ordering/spacing changes. |
| tests/integration/utils/test_uuid.py | Minor assert formatting + import ordering. |
| tests/integration/models/test_operations.py | Import ordering changes. |
| tests/integration/models/test_models.py | Import ordering changes. |
| tests/integration/io/test_datasource_sac.py | Import ordering changes. |
| tests/integration/core/test_station.py | Import ordering + assert formatting. |
| tests/integration/core/test_snapshots.py | Import ordering changes. |
| tests/integration/core/test_seismogram.py | Import ordering changes. |
| tests/integration/core/test_project.py | Import ordering + assert formatting. |
| tests/integration/core/test_event.py | Import ordering + assert formatting. |
| tests/integration/core/test_data.py | Import ordering + assert formatting. |
| tests/functional/test_tui.py | Updates TUI tests to explicitly select an event (no default fallback). |
| tests/functional/test_cli_snapshots.py | Assert formatting + import spacing changes. |
| tests/functional/test_cli_sampledata.py | Assert formatting + import spacing changes. |
| tests/functional/test_cli_project.py | Import ordering + assert formatting changes. |
| tests/functional/test_cli_parameters.py | Assert formatting + import spacing changes. |
| tests/functional/test_cli_basic_ops.py | Import ordering + assert formatting changes. |
| src/aimbat/app.py | Import ordering. |
| src/aimbat/_config.py | Import ordering/spacing changes. |
| src/aimbat/db.py | Adds SQLite connect args + sets WAL mode; updates pragma docstring. |
| src/aimbat/logger.py | Import ordering. |
| src/aimbat/_types/_sqlalchemy.py | Import ordering/grouping. |
| src/aimbat/_types/_pydantic.py | Import ordering/grouping. |
| src/aimbat/_types/_event.py | Import ordering/grouping. |
| src/aimbat/_types/_seismogram.py | Import ordering/grouping. |
| src/aimbat/models/_readers.py | Import ordering/grouping. |
| src/aimbat/models/_parameters.py | Import ordering/grouping. |
| src/aimbat/models/_models.py | Import ordering/grouping. |
| src/aimbat/models/init.py | Import order swap for module re-exports. |
| src/aimbat/io/_base.py | Import ordering/grouping. |
| src/aimbat/io/_data.py | (Context) relied upon by reordered imports; unchanged content. |
| src/aimbat/io/json.py | Import ordering + register import reorder. |
| src/aimbat/io/sac.py | Import ordering/grouping. |
| src/aimbat/io/init.py | Reorders module imports (json/sac imported before _data). |
| src/aimbat/utils/_json.py | Import spacing/grouping. |
| src/aimbat/utils/_sampledata.py | Import ordering; exposes __all_ typo (see comments). |
| src/aimbat/utils/_style.py | Import ordering/grouping. |
| src/aimbat/utils/_uuid.py | Import ordering/grouping. |
| src/aimbat/core/init.py | Reorders star-imports. |
| src/aimbat/core/_data.py | Import ordering/grouping. |
| src/aimbat/core/_default_event.py | Tweaks error message and default-event switching behaviour. |
| src/aimbat/core/_event.py | Import ordering/grouping. |
| src/aimbat/core/_iccs.py | Adds build_iccs_from_snapshot() and adjusts imports/exports. |
| src/aimbat/core/_project.py | Rewraps SQL trigger text blocks for formatting. |
| src/aimbat/core/_seismogram.py | Import ordering/grouping. |
| src/aimbat/core/_snapshot.py | Import ordering/grouping. |
| src/aimbat/core/_station.py | Import ordering/grouping. |
| src/aimbat/_tui/modals.py | Removes default-event toggling from event switcher; updates UI hints/return type. |
| src/aimbat/_tui/app.py | Requires explicit event selection in TUI; removes default-event polling/markers; improves “no event selected” UX. |
| src/aimbat/_cli/common.py | Import ordering; minor internal import placement changes. |
| src/aimbat/_cli/align.py | Import ordering. |
| src/aimbat/_cli/data.py | Import ordering. |
| src/aimbat/_cli/event.py | Clarifies event default semantics for CLI; contains docstring grammar nit. |
| src/aimbat/_cli/pick.py | Import ordering. |
| src/aimbat/_cli/plot.py | Import ordering. |
| src/aimbat/_cli/project.py | Import ordering/grouping. |
| src/aimbat/_cli/seismogram.py | Import ordering/grouping. |
| src/aimbat/_cli/shell.py | Import ordering; shell continues to support shell-local context with default fallback. |
| src/aimbat/_cli/snapshot.py | Import ordering/grouping. |
| src/aimbat/_cli/station.py | Import ordering/grouping. |
| src/aimbat/_cli/utils/app.py | Import ordering. |
| src/aimbat/_cli/utils/sampledata.py | Import ordering. |
| from aimbat import settings | ||
| from aimbat.logger import logger | ||
|
|
||
| __all_ = ["delete_sampledata", "download_sampledata"] |
There was a problem hiding this comment.
__all_ looks like a typo; Python uses __all__ (double underscore on both sides). As written, this variable is unused and can mislead readers/tools that expect the standard name—please rename it to __all__ or remove it if not needed.
| """Select the event to be default for processing.""" | ||
| """Select default event for CLI commands. | ||
|
|
||
| Sets an event to be used by default in when no explicit event ID is given. |
There was a problem hiding this comment.
Docstring grammar: "used by default in when" reads incorrectly. Suggest rewording to something like "used by default when no explicit event ID is given".
Suggested change
| Sets an event to be used by default in when no explicit event ID is given. | |
| Sets an event to be used by default when no explicit event ID is given. |
Comment on lines
+30
to
+32
| The default event is a CLI-only convenience. The [Terminal UI](tui.md), | ||
| [Graphical UI](gui.md), and [Shell](shell.md) each maintain their own event | ||
| selection independently. |
There was a problem hiding this comment.
This section says the Shell maintains its own event selection independently, but the shell implementation still explicitly falls back to the DB default event when its local context is unset (see event switch docs / shell_event_id: None means "use DB default event"). Consider clarifying this here so users understand the default event can still affect shell behaviour when no shell event is selected.
Suggested change
| The default event is a CLI-only convenience. The [Terminal UI](tui.md), | |
| [Graphical UI](gui.md), and [Shell](shell.md) each maintain their own event | |
| selection independently. | |
| The default event is primarily a CLI convenience. The [Terminal UI](tui.md) and | |
| [Graphical UI](gui.md) each maintain their own event selection independently of | |
| the CLI. The [Shell](shell.md) also tracks its own current event; when no shell | |
| event has been selected in the current session it will fall back to the project | |
| default event. |
Comment on lines
26
to
+49
| @@ -32,9 +43,10 @@ | |||
| def set_sqlite_pragma( | |||
| dbapi_connection: sqlite3.Connection, connection_record: ConnectionPoolEntry | |||
| ) -> None: | |||
| """Enable foreign key support for each new SQLite connection.""" | |||
| """Enable foreign key and WAL support for each new SQLite connection.""" | |||
| cursor = dbapi_connection.cursor() | |||
| cursor.execute("PRAGMA foreign_keys=ON") | |||
| cursor.execute("PRAGMA journal_mode=WAL") | |||
There was a problem hiding this comment.
The PR title/description focus on default-event behaviour in TUI/GUI, but this change also modifies the global DB engine configuration (adds SQLite connect_args and forces PRAGMA journal_mode=WAL). Please either justify these operational changes in the PR description or split them into a separate PR, since they can affect deployment/runtime behaviour (e.g., extra WAL files, connection/threading semantics).
Using default event in tui and gui is confusing since we select which event to work on there anyway.
smlloyd
force-pushed
the
use-default-only-for-cli
branch
from
e45f3ec to
2c4d830
Compare
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.
Using default event in tui and gui is confusing since we select which event to work on there anyway.
📚 Documentation preview 📚: https://aimbat--227.org.readthedocs.build/en/227/