Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #222 +/- ##
==========================================
+ Coverage 93.39% 93.54% +0.14%
==========================================
Files 45 46 +1
Lines 1893 1936 +43
==========================================
+ Hits 1768 1811 +43
Misses 125 125 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR adds JSON data source support alongside SAC, refactors the codebase structure for consistency, and adds documentation on core concepts. The main change is a plugin-based I/O architecture where data source modules register their capabilities upon import.
Changes:
- Add JSON datasource (
io/json.py) with support for station-only and event-only imports viaJSON_STATIONandJSON_EVENTdata types - Rename
cli/→_cli/andaimbat_types/→_types/for internal module naming consistency - Move
DataTypeenum fromaimbat_types/_data.pytoio/_data.pyand move SQLAlchemy type decorators frommodels/_sqlalchemy.pyto_types/_sqlalchemy.py - Refactor
core/_data.pyto support optionalstation_idandevent_idparameters, allowing SAC files to link to pre-existing JSON-imported stations/events - Add UUID prefix resolution helpers in
_cli/common.pyfor cleaner CLI parameter handling - Reorganise tests to mirror the new
src/structure - Add
docs/first-steps/core-concepts.mdexplaining AIMBAT's motivation and semi-automatic workflow
Reviewed changes
Copilot reviewed 49 out of 62 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/aimbat/io/init.py | Remove automatic SAC import; update documentation (but critical bug: neither SAC nor JSON are imported anywhere) |
| src/aimbat/io/_data.py | New file: DataType enum moved from aimbat_types |
| src/aimbat/io/sac.py | Renamed from _sac.py; registers SAC support on import |
| src/aimbat/io/json.py | New file: JSON station/event creation with registration |
| src/aimbat/core/_data.py | Refactored _add_datasource → _process_datasource; add station_id/event_id support; early UUID validation |
| src/aimbat/_cli/data.py | Renamed from cli/; add --use-station and --use-event parameters; enhanced help text |
| src/aimbat/_cli/common.py | Renamed from cli/; add UUID prefix resolution helpers and shared parameter factories |
| src/aimbat/_cli/*.py | Renamed from cli/; updated imports and enhanced docstrings |
| src/aimbat/_types/*.py | Renamed from aimbat_types/; moved _sqlalchemy.py from models/ |
| src/aimbat/models/_models.py | Update imports for renamed modules |
| tests/unit/io/test_json_sources.py | New comprehensive tests for JSON data source |
| tests/integration/utils/test_uuid.py | New tests for UUID prefix resolution |
| tests/integration/core/test_data.py | Moved and expanded from test_data_io.py; add JSON integration tests |
| tests/integration/core/test_event.py | Consolidated active event tests from test_active_event.py |
| tests/integration/models/test_operations.py | New comprehensive ORM relationship and cascade tests |
| docs/first-steps/core-concepts.md | New page explaining AIMBAT's motivation and workflow |
| .github/copilot-instructions.md | Update module paths (some outdated references remain) |
| zensical.toml | Update copyright and API reference paths |
| - Use `PydanticTimestamp` / `PydanticTimedelta` (from `aimbat.aimbat_types`) for pandas-compatible time fields in models | ||
| - Use `PydanticTimestamp` / `PydanticTimedelta` (from `aimbat._types`) for pandas-compatible time fields in models | ||
| - Use `PydanticNegativeTimedelta` / `PydanticPositiveTimedelta` for constrained sign validation | ||
| - Use `SAPandasTimestamp` / `SAPandasTimedelta` (from `aimbat.models._sqlalchemy`) as the `sa_type` in SQLModel fields |
There was a problem hiding this comment.
The import path aimbat.models._sqlalchemy is incorrect after this PR moved the file to _types/_sqlalchemy.py. Update to: "Use SAPandasTimestamp / SAPandasTimedelta (from aimbat._types) as the sa_type in SQLModel fields"
| - Use `SAPandasTimestamp` / `SAPandasTimedelta` (from `aimbat.models._sqlalchemy`) as the `sa_type` in SQLModel fields | |
| - Use `SAPandasTimestamp` / `SAPandasTimedelta` (from `aimbat._types`) as the `sa_type` in SQLModel fields |
| from ._data import * | ||
| from ._base import * |
There was a problem hiding this comment.
The SAC and JSON data source modules are never imported anywhere in the production codebase, which means their registration code will never run. This breaks SAC file support (the primary data format) in AIMBAT.
According to the docstring in io/__init__.py, "SAC support (aimbat.io.sac) is available but not loaded automatically; import it explicitly to enable it." However, there is no explicit import anywhere.
The registration happens via side effects when the modules are imported (see lines 87-91 in io/json.py and lines 145-149 in io/sac.py). Without importing these modules, the register_* functions are never called, so DataType.SAC and DataType.JSON_STATION / DataType.JSON_EVENT won't work.
Fix: Import both modules in io/__init__.py to ensure they register on package load:
from ._data import *
from ._base import *
# Import data source modules to register their capabilities
from . import sac, json # noqa: F401Alternatively, import them in models/_models.py before they're needed, or in db.py as part of initialization.
| @@ -0,0 +1,168 @@ | |||
| """Unit tests for aimbat.io._json.""" | |||
There was a problem hiding this comment.
The module docstring refers to aimbat.io._json but the actual module name is aimbat.io.json (as shown by the import on line 8). The underscore prefix convention is typically used for internal/private modules, but json.py does not have an underscore prefix.
Update the docstring to: """Unit tests for aimbat.io.json."""
| │ └── _sqlalchemy.py # SAPandasTimestamp / SAPandasTimedelta type decorators | ||
| ├── aimbat_types/ # Custom Pydantic types (PydanticTimestamp, enums for parameters) | ||
| ├── _types/ # Custom Pydantic types (PydanticTimestamp, enums for parameters) | ||
| ├── io/ # File I/O — _base.py defines abstract base; _sac.py implements SAC via pysmo |
There was a problem hiding this comment.
The comment refers to _sac.py but the file was renamed to sac.py (without underscore) in this PR. Update to: "File I/O — _base.py defines abstract base; sac.py implements SAC via pysmo"
| ├── io/ # File I/O — _base.py defines abstract base; _sac.py implements SAC via pysmo | |
| ├── io/ # File I/O — _base.py defines abstract base; sac.py implements SAC via pysmo |
| ├── _cli/ # CLI command definitions (thin layer, delegates to core/) | ||
| ├── core/ # Business logic: ICCS/MCCC algorithms, event/seismogram ops | ||
| │ ├── _active_event.py # Manages the single active event constraint | ||
| │ ├── _data.py # SAC ingestion entry point |
There was a problem hiding this comment.
The comment describes core/_data.py as "SAC ingestion entry point" but after this PR it handles generic data ingestion for all data types (SAC, JSON_STATION, JSON_EVENT). Update to something like: "Data ingestion entry point for all data types"
| │ ├── _data.py # SAC ingestion entry point | |
| │ ├── _data.py # Data ingestion entry point for all data types |
smlloyd
force-pushed
the
json-datasource
branch
from
667c619 to
aa34054
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| from aimbat.db import engine | ||
| from aimbat.core import add_data_to_project | ||
|
|
||
| global_parameters = global_parameters or GlobalParameters() |
There was a problem hiding this comment.
is used for global --debug flag
| from aimbat.core import print_data_table | ||
|
|
||
| table_parameters = table_parameters or TableParameters() | ||
| global_parameters = global_parameters or GlobalParameters() |
| from aimbat.core import dump_data_table_to_json | ||
| from rich import print_json | ||
|
|
||
| global_parameters = global_parameters or GlobalParameters() |
- Add JSON datasource (io/json.py) alongside SAC datasource (io/sac.py) - Rename cli/ → _cli/ and aimbat_types/ → _types/ for consistency - Move data I/O logic to io/_data.py - Reorganise tests to mirror new src layout - Add core-concepts documentation page
smlloyd
force-pushed
the
json-datasource
branch
from
aa34054 to
ed4a16d
Compare
📚 Documentation preview 📚: https://aimbat--222.org.readthedocs.build/en/222/