feat: Use infrastore as the backend for time series and component/supplemental_attribute associations - #162
Draft
daniel-thom wants to merge 48 commits into
Draft
feat: Use infrastore as the backend for time series and component/supplemental_attribute associations#162daniel-thom wants to merge 48 commits into
daniel-thom wants to merge 48 commits into
Conversation
Components, supplemental attributes, and time series metadata now use monotonically-increasing integer IDs as their primary identifier. Legacy UUIDs are preserved as `legacy_uuid` for backward compatibility. Key changes: - InfraSysBaseModelWithIdentifiers: new id field (int), legacy_uuid for BC - IDManager: tracks next available ID, supports advance_past() for serialized data with pre-assigned IDs - SQLite tables use INTEGER foreign keys with referential integrity - ComponentAssociations, SupplementalAttributeAssociations, and TimeSeriesMetadataStore all migrated to integer ID columns - Migration logic for legacy serialized data and existing SQLite schemas - New get_by_id() methods on ComponentManager, SupplementalAttributeManager - SerializedComponentReference uses integer id instead of UUID - Fixed double-negative bug in metadata_store_needs_migration - copy.deepcopy replaces manual model_dump() reconstruction - Backward-compat alias InfraSysBaseModelWithIdentifers preserved
- Fix ComponentManager.remove() index cleanup only triggering on empty container (P1 bug) - Fix get_by_label() to try name-based lookup before ID for numeric labels (P1 ambiguity) - Add owner_category to time series unique index to prevent false collisions across owner types - Extract migration functions to src/infrasys/utils/migrations.py - Raise NotImplementedError for deprecated get_component_uuids_with_attribute - Merge duplicate query strings in supplemental attribute associations - Add Field descriptions to SerializedComponentReference - Add cascade warning when removing components with time series - Add migration path tests and remove-index cleanup test
…nent associations Covers: - SerializedComponentReference.uuid property (success + error) - get_class_and_name_from_label with UUID and unknown string labels - ComponentAssociations.clear operation - list_child_components/list_parent_components with type filter - get_by_label with UUID-based labels
- fix duplicate list_time_series_metadata call in remove_component - widen sql() params type from Sequence[str] to Sequence[Any] - guard MAX(*_id) queries with column existence checks for legacy DBs - add 17 tests covering component associations, ID management, migrations, supplemental attribute queries, and time series metadata normalization
ts-store moved the association `name` (required) and `scaling_factor_multiplier` (optional) off the `add_time_series` call and onto the time series constructors. Pass `metadata.name` to RustSingleTimeSeries / RustNonSequentialTimeSeries and drop the now-rejected `name=` keyword from add_time_series. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface the time-series-store backend's NetCDF compression policy as System
kwargs: time_series_compression ("deflate"/"none"), time_series_compression_level
(0-9), and time_series_shuffle. They flow System -> TimeSeriesManager ->
TimeSeriesStoreStorage.create_* -> TimeSeriesStore.create, defaulting to the
prior behavior (DEFLATE level 3 + shuffle). The memory backend ignores them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… type, replace asserts with domain exceptions Agent-Logs-Url: https://github.com/NatLabRockies/infrasys/sessions/070e4fb1-22bd-4d30-8a82-321e1f62f210 Co-authored-by: pesap <2238996+pesap@users.noreply.github.com>
# Conflicts: # src/infrasys/time_series_manager.py # tests/test_serialization.py
The rust core replaced time-series owner identity (owner_uuid: str) with owner_id: i64, matching the integer IDs introduced on feat/ids. Switch the storage layer to pass metadata.time_series_id as owner_id for adds and key lookups, and fix the dependency pin to point at ../time-series-store. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…identity The Rust time-series-store core now owns time-series identity (content hash plus the owner/name/features association key), so infrasys no longer keeps its own parallel metadata store or assigns UUIDs/integer IDs to time series. - Remove the Python TimeSeriesMetadataStore, in-memory backend, storage base class, and legacy UUID->ID DB migrations. - Make TimeSeriesStoreStorage the single backend: it owns data plus an in-memory index of lightweight records, with the Rust store as the persistent authority for data and associations. - Reparent TimeSeriesData to InfraSysBaseModel (no id/uuid) and delete the TimeSeriesMetadata model hierarchy; list_time_series_metadata now returns TimeSeriesKey objects. Extract SingleTimeSeries slicing into single_time_series_range. - Define TimeSeriesCounts via content-addressed array groups: time_series_count is the number of unique stored arrays and reference_count is total owner associations (surfacing dedup/sharing). - Drop the MEMORY storage type and convert_storage; normalization is no longer echoed back on retrieval (it was never persisted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s-store backend Wire Deterministic forecasts through the Rust time-series-store adapter, which previously raised NotImplementedError for anything but SingleTimeSeries and NonSequentialTimeSeries. - Add explicit Deterministic forecasts via add_time_series; read them back with get_time_series(..., time_series_type=Deterministic). - Add System.transform_single_time_series(horizon, interval) to derive "perfect forecasts" from stored SingleTimeSeries (store-wide, mirrors InfrastructureSystems.jl). Forecast windows are materialized in Rust. - A Deterministic query matches both explicitly-stored and transform-derived (DeterministicSingleTimeSeries) records; reads always return a Deterministic. - Transpose between the infrasys (window_count, horizon_steps) layout and the store's (horizon_steps, count) layout on add/get. - Reuse DeterministicTimeSeriesKey and add AbstractDeterministic.length. Also refresh documentation left stale by the time-series-store and integer-id migrations: serialization JSON examples now use integer ids, the removed storage backends / in-memory option are dropped, and dangling DeterministicSingleTimeSeries / from_single_time_series references are fixed. Add developer instructions for building the Rust time-series-store extension with uv. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add explicit per-window value checks for forecasts derived via transform_single_time_series, beyond the existing shape/round-trip coverage: - Parametrized cases (overlapping, max-overlap, contiguous, partial final stride) assert each window i equals underlying[i*interval : i*interval + horizon], using random non-monotonic data so an orientation/transpose bug cannot pass by symmetry. - Verify window values survive a to_json/from_json round trip. - Verify pint units are preserved through the transform. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add bulk-add and rollback tests for Deterministic and NonSequentialTimeSeries, closing the gap where only SingleTimeSeries exercised the open_time_series_store transaction path. Add negative tests asserting the Deterministic vs DeterministicSingleTimeSeries mutual-exclusion guard in both directions (explicit add after transform, transform after explicit add). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace infrasys's own SQLite association tables with the association API
that now lives in the Rust-backed `time_series_store` package. Both
supplemental attribute associations and component parent/child
associations are stored in the time series store's SQLite catalog
(`<store>.nc.sqlite`), which is saved and restored with the system.
Deleted:
- src/infrasys/supplemental_attribute_associations.py
- src/infrasys/component_associations.py
- src/infrasys/utils/metadata_utils.py
- src/infrasys/utils/sqlite.py (every function lost its last caller)
SupplementalAttributeManager and ComponentManager now take the store
instead of a sqlite3.Connection, so System.__init__ builds the time
series manager first and hands its store to both. `open_metadata_store`
no longer wraps a SQLite transaction; the store exposes no transaction
primitive, so it snapshots the association rows on entry and restores
them verbatim if an exception escapes. Duplicate supplemental attribute
adds are now detected by the store, which raises
DuplicateAssociationError; that is translated back to ISAlreadyAttached
so the public contract is unchanged.
Component associations used to live in a private in-memory database that
was rebuilt on every deserialization. They are now persisted, so
ComponentManager.add skips the association write when
deserialization_in_progress is set, and a component that references the
same child from two fields is de-duplicated before the bulk insert.
BREAKING CHANGE: clean break on persistence. System.DB_FILENAME
("time_series_metadata.db"), the backup/restore of that file, and
System._con are gone. Supplemental attribute associations now travel
inside the time series store artifact instead of a sidecar SQLite file,
so systems saved by an earlier build will no longer load. This is
intended. TimeSeriesStorageContext.metadata_conn was removed for the
same reason.
Also removed the UUID-era association schema migration
(migrate_legacy_uuid_table / migrate_legacy_association_schema), which
no longer has an on-disk format to migrate from.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Rust store package was renamed from time-series-store to castore, and its main Python class TimeSeriesStore is now exported as Store. Repoint the dependency (pyproject + uv source), switch imports to `from castore import ...`, and use `Store` directly in place of the old `TimeSeriesStore` name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`[tool.uv.sources]` pinned infrastore to `../castore/crates/infrastore-py`,
a sibling working copy. CI checks out only this repository, so `../` is
empty there and every job died in `uv sync` before running a single hook
or test:
error: Failed to generate package metadata for `infrastore==0.1.0`
Caused by: Distribution not found at: file:///home/runner/work/infrasys/castore/...
The path was stale locally too, since that checkout is now `../infrastore`.
infrastore 0.1.1 is on PyPI as abi3-py311 wheels for macOS arm64,
manylinux x86_64, and win_amd64, which covers the full test matrix, so
the source override can go entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`serialize` flushed the store and then copied `time_series_store.nc` and
its SQLite catalog while the Rust store still held them open. POSIX
allows that; Windows denies the read, so all six Windows test jobs failed
26 tests with:
PermissionError: [Errno 13] Permission denied
src/infrasys/time_series_store_storage.py:596: in _copy_store
shutil.copyfile(src, dst)
Close the store around the copy and reopen it afterwards, preserving the
read-only mode. The reopen is in a `finally`, so a failed copy leaves the
storage usable instead of stranded on a closed handle. Only the live
directory takes this path; an explicit `src` elsewhere is not held open.
Closing yields a new handle, and `ComponentManager` and
`SupplementalAttributeManager` had captured the old one at construction,
which would leave them raising "store is closed" after the first save.
Both now resolve it from the storage on each access, so the swap is
invisible to them.
These jobs had never run before: they are gated behind `needs:
pre-commit`, which failed at dependency installation on every prior push.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…store refactor: store associations in the Rust time-series-store
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
==========================================
+ Coverage 94.82% 96.37% +1.54%
==========================================
Files 61 52 -9
Lines 6072 5988 -84
==========================================
+ Hits 5758 5771 +13
+ Misses 314 217 -97 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
daniel-thom
changed the title
daniel-thom
marked this pull request as draft
July 26, 2026 15:14
…ydration The Rust store exposes add_time_series_bulk/bulk_read, but nothing here called them: every write paid one catalog transaction and every read one decompress pass. On a file-backed store that is roughly two orders of magnitude of avoidable work. - open_time_series_store now buffers additions and commits them in one batch instead of yielding None. The index is updated as each series is staged so metadata queries inside the block are unchanged; anything that must touch the store flushes first. Staged additions are dropped without a store round trip when the block raises, which also simplifies rollback_to. - add_time_series validates all owners before mutating anything and commits them together, so a duplicate on the last owner no longer leaves the earlier ones attached. - list_time_series reads through bulk_read, grouping records by time range since one range applies to the whole batch. get_time_series is split into _plan_read/_build_result so both paths share the conversion logic. - Store keys are cached on _StoredSeries: from the keys add_time_series_bulk returns, from a single list_keys() call during rehydrate, and memoized on the fallback scan. Reads no longer rescan an owner's keys. - _record_from_store reads initial_timestamp, horizon, interval, and count off the store's metadata record, which already carries them, so rehydrate no longer reads array data. 100 series of 8760 points, file-backed store: adds 38.7s -> 0.3s, from_json 13.8s -> 0.01s, list_time_series 4.9s -> 0.06s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Simulations step through time and need every component's value at one
timestamp. Everything the time series API offered was series-oriented, so
the only ways to drive a stepping loop were to hold every array in memory
or to re-read per component per step. The store's reader API is built for
exactly this and was unexposed.
- System.build_time_series_reader(resolution, ...) returns a TimeSeriesReader
whose read(timestamp) is {component id: value}. A dict keeps the caller
free of any iteration order; read_columns() is the escape hatch that hands
back the store's arrays with no per-value Python objects.
- System.build_forecast_reader(resolution, ...) returns a ForecastReader whose
read(timestamp) is {component id: window array}. Slot deduplication is
exposed rather than hidden: num_slots, slots, and components_by_slot()
describe the collapse, read_slots() reads one window per slot, and read()
hands every component in a slot the same array object instead of a copy.
A fleet sharing one profile therefore materializes one window, not N.
- Both readers filter on name, name_glob, component_type, and features, expose
the stored units so raw magnitudes are not silently mistaken for quantities,
and flush any open add batch at build time.
_as_utc/_as_naive_utc move to utils.time_utils as as_utc/as_naive_utc so the
reader module can convert timestamps without importing the storage backend.
5000 components x 168 steps: 0.16s stepping through readers (0.95 ms/step,
or 0.45 ms/step via read_columns) against 0.52s just to preload the arrays,
which is the part that stops scaling. Transformed forecasts over the same
fleet collapse 5000 components to 2501 slots.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The readers landed with docstrings only. The rendered docs still described a purely series-oriented API, so a user driving a simulation loop had no way to discover them and the obvious-looking paths --- preload every array, or get_time_series per component per step --- are the slow ones. - A how-to walks the stepping loop end to end: filters, the two build-time failures (non-uniform grid, empty match), the snapshot semantics, read_columns as the escape hatch, units, and forecast slot deduplication including the shared array object that makes read() cheap. Every snippet and its output was executed against the repo rather than written by hand. - reference/api/time_series.md autoclasses TimeSeriesReader and ForecastReader, which were absent from the API reference entirely; only the System build methods were reachable, via the System autoclass. - explanation/time_series.md gains a section on the series-oriented vs cross-sectional distinction, which is where a reader of that page would expect to find out these exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
open_time_series_store yielded a TimeSeriesStorageContext that callers threaded into add/get, but the object was empty --- one field, data_context, always None --- and the batching it appeared to control lived on the storage instance. Passing the context, omitting it inside the block, and running with no block produced [3], [3], and [1,1,1] bulk writes: the token did nothing. Two defects followed from storage owning the batch. rollback_to undid "everything added since a snapshot of the shared index", so a second open batch would have had its work reverted. And eight methods called _flush_pending() without taking a context, so any operation anywhere drained whichever batch happened to be open. The context is now the transaction object. It owns the staged additions, the index entries they will produce, and the record of what it has already written so a failure undoes exactly its own work. Storage keeps no batch state and holds no reference to any context; ownership points one way. The same measurement now reads [3], [1,1,1], [1,1,1] --- passing the context is what puts a call in the batch. - New time_series_context module holds TimeSeriesStorageContext plus the _StoredSeries and _PendingAdd dataclasses moved from storage. flush() writes one bulk call, discard() drops staged work and removes anything already flushed. - Staged additions resolve only through the context that staged them, so two open batches never observe or disturb each other. snapshot_index/rollback_to are gone: staging no longer dirties the shared index, so rollback is "drop the context". - Every public method takes context=None and falls back to a transient context that commits the single operation, leaving existing call sites valid. system.py gains the parameter on the eleven methods that lacked it, including to_json, which without it serializes committed state only. - Storage owns every read of its index: one merge point resolves committed against a context's staged entries. Drops the dead mode parameter and the FileMode alias, both ignored by the backend, where mode="r" implied a read-only guarantee it never gave. Fixes a latent bug carried over from rollback_to: the OwnerCategory was reconstructed from its name with a bare else, so an unrecognized category silently became SupplementalAttribute and removed the wrong association. The one behavior change: inside a block, a call that omits the context sees committed state only. Four tests asserted the old ambient visibility and now pass their context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…paths - TimeSeriesManager.close probed for close/dispose methods that TimeSeriesStoreStorage never defined, so System.close never released the Rust store's file handles. Give the storage a real close() and call it directly. - remove_component and remove_supplemental_attribute filtered on the default SingleTimeSeries type, orphaning Deterministic and NonSequentialTimeSeries associations under dead owner ids, and their per-key removal loop crashed when one series' features were a subset of another's. Both now remove everything with one match-all call. - Accept time_series_type=None as a match-any filter in has/list/remove (previously an AttributeError). - Storage removal now resolves every matching association first and removes them via one remove_time_series_bulk call, updating the index only after the store accepts, so a failure cannot diverge the two. - Add a TODO for rolling back removals on context discard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add an explanation page describing how bulk adds, bulk reads, and the cross-sectional readers work, with colored mermaid diagrams of the write/read paths and the reader build/step split. Requires the new sphinxcontrib-mermaid extension. Also make a clean build warning-free: wire the orphaned storage_backends, save_system, and benchmarks pages into their toctrees, drop the nonexistent _static path, start system_json at H1, and disable the JSON schema dropdown for NonSequentialTimeSeries, whose ndarray field cannot be schema-serialized. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The context owned a _committed log and undid a failed block by replaying compensating removals through the store. That could restore a catalog row but never an array: outside a transaction the store frees an array as soon as its last association goes, so a removal was irreversible and discard() carried a TODO saying so. It also made every mid-block flush a one-way door -- a read, a counts call, or a reader build inside a block quietly converted "drop the buffer" into "replay removals and hope". infrastore now has transactions spanning operations, so rollback is one call. open_time_series_store begins one and commits or rolls it back on exit. _committed and undo_committed are gone. Removals roll back, because the store defers freeing an array to the outermost commit; the TODO goes with them. An early flush costs nothing in recoverability, since the write lands inside the transaction. The client-side add buffer stays. Batching is what buys block-sized NetCDF writes and feature-set dedup, and a transaction deliberately does not provide it. The staged overlay stays too, narrowed to the one thing the store cannot answer: an addition still in the buffer has not reached the store, so a duplicate against it would go undetected until the flush. A context created for a single operation does not begin a transaction -- that operation is already atomic, and taking a write lock for it would be wasted work and would fail outright on a read-only store. discard() rebuilds the index from the store after rolling back, since entries recorded as work was flushed describe a catalog state that no longer exists. A rollback that itself fails is logged, not raised, so the error that caused the unwind is the one the caller sees. Two behavior changes, both consequences of the mechanism: - Transactional blocks nest LIFO. SQLite savepoints are a stack, so an inner block must finish before its enclosing one; two interleaved blocks that each commit or discard on their own schedule are not supported. Non-transactional batches from new_context() are unaffected and stay concurrent. - Serializing inside an open block raises. Copying the artifact closes and reopens it, which would discard the transaction, and a durable copy of state that may still roll back is not coherent. to_json moves outside the block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…transaction The old name described an implementation that no longer exists: it dates from when the call opened and held a file handle. Nothing is opened now -- the store is already open -- and the name says nothing about the property that matters most since the block became a store transaction, which is that it is atomic and rolls back, removals included. `time_series_transaction` names the guarantee rather than a mechanism, and matches the vocabulary of the layer below, where infrastore exposes `Store.transaction()`. Chosen jointly with InfrastructureSystems.jl so the two packages read the same. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sh large batches Resolve the batch-API design debate in favor of a facade: time_series_transaction now yields a TimeSeriesTransaction whose methods mirror System's and route through the batch, replacing the context= kwarg that every System time series method (and to_json) previously threaded. The context kwarg was easy to omit silently, and the opaque token had no other user-facing purpose; TimeSeriesStorageContext remains as internal plumbing. to_json also loses context= — serializing inside an open transaction already raises unconditionally, so the parameter was vestigial. The same reasoning applies one layer down, so the context is now the *receiver* for the operations rather than a parameter to them: ctx.add_time_series(ts, gen, **features) instead of storage.add_time_series(ts, gen, context=ctx, **features). Twelve operations moved onto TimeSeriesStorageContext; the storage-side implementations became private and take their context positional-only; and TimeSeriesTransaction binds the manager to its batch once at construction (TimeSeriesManager.bind_context) instead of forwarding context= on every call. That closes a live collision rather than a theoretical one. A time series feature named "context" bound the plumbing parameter and died with AttributeError: 'str' object has no attribute 'check_owns' — plumbing had no business reserving a name in the namespace a caller owns. It also makes a context reaching the wrong storage unrepresentable instead of merely checked: check_owns survives at one site, bind_context, rather than eight. A transaction now also flushes on its own at AUTO_FLUSH_THRESHOLD (10,000) staged additions or AUTO_FLUSH_BYTES (256 MiB) of staged array data, whichever first, so adding hundreds of thousands of series holds bounded memory. The count keeps each bulk write near the store's chunk layout sweet spot (chunk width equals batch width; 10,000 f64 series produce 80 KiB chunks against infrastore's 1 MiB cap, at ~2% throughput cost vs an unlimited batch), while the byte limit bounds memory when individual arrays are long, which a count cannot. Auto-flushed work lands inside the open store transaction and rolls back with the block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`units` became a user-declared, immutable, non-filterable label on every time
series struct in InfrastructureSystems.jl, InfraStore.jl, and the Rust core.
infrasys was left out, deliberately: it already writes the store's `units`
column, but with a different payload and different provenance.
Today `_units_from_data` derives a `QuantityMetadata` blob
(`{module, quantity_type, units}`) from `pint.Quantity` data, serializes it
into the column, and `_build_result` reads it back to rehydrate the original
quantity subclass on every read. So the column holds a JSON object where the
other bindings now expect a bare label, and infrasys infers the value where
the agreed design has the user declare it.
That is not a naming nit: one column cannot hold both payloads, a store
written by infrasys and read by IS.jl surfaces a blob where a label belongs,
and `QuantityMetadata` is load-bearing -- `module` and `quantity_type` are
what let a read reconstruct a custom `BaseQuantity` subclass.
The note records the current behavior with line references, three options
(move the blob to the unused `ext` column, flatten to a string and lose
subclass reconstruction, or opt infrasys out), and the questions still open --
including precedence when a user declares `units=` and passes a
`pint.Quantity` whose units disagree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The store gained `quantity_kind` and `unit_system` and renamed `ext` to `application_data` (data format 0.15.0). Nothing in infrasys breaks -- its suite passes unchanged, because it never used the renamed column -- but the change materially alters the options this note was left open on. Option A got cheaper and splits three ways: `application_data` is now documented as the home for exactly this kind of package-owned payload, and `QuantityMetadata`'s three fields have three natural homes instead of one opaque blob. Seven of the eight classes in `quantities.py` are already QUDT `QuantityKind` local names verbatim, so `quantity_type` maps onto `quantity_kind` and only `module` stays binding-private. `Current` is QUDT's `ElectricCurrent`; that one name is the whole cost of adopting the vocabulary. This decides nothing: open questions 2 and 3 (precedence between a declared `units=` and a `pint.Quantity`, and whether infrasys exposes `units=` at all) are untouched. `unit_system` also does not map cleanly -- infrasys spells the device base `DEVICE_BASE` where the store spells it `component_base`, and its third value `SYSTEM_BASE` has no storage spelling at all. Also replaces the `infrastore-core` line-number citations with symbol names. The note itself flagged those as a contract that breaks silently on refactor, and 0.15.0 is what broke them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upgrade to infrastore v0.9.0 and stop flattening timestamps to naive UTC. infrasys used to force every timestamp to aware UTC on write and strip it back to naive on read. That shim existed only because the old binding *refused* a naive datetime; the cost was that it silently destroyed any zone a caller supplied and told the catalog every series was UTC, which a Julia or CLI reader of the same store then believed. v0.9.0 records how a series' timestamps were spelled -- an instant in UTC, an instant at a fixed offset, an instant in a named IANA zone, or a wall clock naming no instant -- and hands the same spelling back. So the shim is obsolete: timestamps now cross the boundary as written. A naive datetime is a wall clock and comes back naive; an aware one comes back aware in the same zone. One system can hold both, because the spelling is per series. Three parts of this were more than deleting a conversion: - Rehydration. The catalog renders the instant and the spelling in separate columns, so `from_catalog_timestamp` reassembles them when the index is rebuilt from a saved store. - Python's datetime arithmetic is *wall clock* whenever both operands share a tzinfo. A grid steps instants, so `initial_timestamp + n * resolution` drifts across a DST transition, and `start_time - initial_timestamp` on a Denver series answers 31 hours where the store means 30. `advance` and `as_instant` do that arithmetic in one frame; without them a slice across 2024-03-10 read back index 31 instead of 30. - Coherence. A reader materializes one timestamp axis, so the store refuses a cohort mixing wall clocks with instants. `zoneless=True|False` is threaded through build_time_series_reader/build_forecast_reader as the constructive half of that rule, and a mismatched `start_time` now raises ISConflictingArguments naming both values rather than leaking a TypeError. NonSequentialTimeSeries no longer pushes aware timestamps through datetime64 (which dropped the offset with a warning) and rejects mixed spellings. `make_timestamps` is unchanged but documented: datetime64 has no room for a zone, so a zoned series' array holds the instants in UTC. BREAKING CHANGE: a naive timestamp is now stored as a wall clock rather than relabelled UTC, and an aware one is returned aware rather than stripped. The store's data format went to 0.17.0, so stores written by earlier infrastore versions are rejected on open; there is no migration, only rewriting. infrasys still does not populate the store's quantity_kind, unit_system, or application_data descriptors -- UNITS_FIELD_NOTE.md tracks that as an open decision and this change does not preempt it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V9S2KTTESCV9Ac9kgFzM2Z
Each manager previously owned its own IDManager starting at 1, so a component and a supplemental attribute could be assigned the same ID. The System now owns a single IDManager and shares it with both, making an ID identify exactly one object whatever its kind. This matches what utils/migrations.py already does when assigning IDs to legacy data. Removed the supplemental_attribute_manager parameter from System. Only from_dict passed it, with the same storage System already builds itself, and it was the one way a caller could hand the system a manager with a divergent ID counter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V9S2KTTESCV9Ac9kgFzM2Z
Ports the fixes from InfrastructureSystems.jl's f3dd1d2ba and c20308b35 that apply here. Both managers mutate their in-memory containers before the store write that persists the change, so a refusal arriving from that write left the system describing a store it no longer agreed with. - Add TimeSeriesStoreStorage.raise_if_read_only(). Every manager in a system writes through the one store -- component parent/child associations and supplemental attribute associations as well as time series -- so that is the shared gate. ComponentManager.add/remove and SupplementalAttributeManager .add/remove/remove_association call it up front rather than discovering the refusal after the containers are updated. Deserialization is exempt: it populates from a store that already holds the rows and writes nothing. System._raise_if_read_only() asks both gates, the store's and the time series manager's (which can be read-only while the store is not), so a removal spanning three managers refuses before the first of them. - remove_component detaches the component's supplemental attributes. An attribute left with no other component holding it is removed from the system, taking its own time series with it. Previously the association rows survived the component and pointed at an id that no longer existed. - Drive the cascade_down cascade from System instead of recursing inside ComponentManager.remove, which now returns the ids it orphaned. A cascaded child had been removed without any of the System-level cleanup, leaking its time series as well as its attributes. The loop re-checks that each id is still stored, since removing one orphan can cascade to another in the same list -- an ISNotStored the old recursion could hit as well. - Route the last-association cascade in remove_supplemental_attribute_from_component back through System, so the attribute's time series go with it. The manager owns no handle on the time series manager, so remove_attribute_from_component becomes remove_association and returns whether the attribute is now unreferenced. - Run the force=False reference check before detaching anything. It lived inside ComponentManager.remove, i.e. after System had stripped the component, so a refused removal left the component in place without its attributes or time series. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXNMsqZBdT19YcfFVv8ch5
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.
Changes
Adopting infrastore brings additional functionality (full Sienna time series support) and loses functionality. We will drop support for time series Arrow files and chronify. The only supported time series backends will be those provided by infrastore - currently NetCDF and in-memory. We will extend infrastore as needed.
TODO
Provide automated migration of existing serialized systems.