Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors timedatamodel to make time-series “identity” and spatial context consumer-owned, leaving timedatamodel with metric-first, pure metadata and univariate series handling (and removing the multivariate table API).
Changes:
- Rename
name→ requiredmetricacrossTimeSeriesandTimeSeriesDescriptor; removelabelsandlocationfrom both. - Remove
TimeSeriesTable(and its repr mixin / docs / tests). - Add pint-backed
validate_unit(unit)and refresh docs/examples/tests; bump version to0.2.1.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Update locked local package version to 0.2.1. |
| timedatamodel/units.py | Add validate_unit() helper for pint-backed unit-string validation. |
| timedatamodel/timeseriestable.py | Removed TimeSeriesTable implementation. |
| timedatamodel/timeseriesdescriptor.py | Make descriptor metric-first; remove labels/location; simplify and keep frozen/slots. |
| timedatamodel/timeseries.py | Make metric required; remove labels/location; update descriptor round-trip and metadata helpers. |
| timedatamodel/_repr.py | Remove location/labels from repr; remove table repr mixin; switch “Name” → “Metric”. |
| timedatamodel/init.py | Stop exporting TimeSeriesTable; keep location primitives exported. |
| tests/test_units.py | Add tests for validate_unit() (pint-gated). |
| tests/test_timeseriestable.py | Remove tests for TimeSeriesTable. |
| tests/test_timeseriesdescriptor.py | Update descriptor tests for metric requirement and new field set. |
| tests/test_timeseries.py | Update tests to use required metric and remove label/location expectations. |
| README.md | Update public-facing API narrative/examples to metric-first and remove table mentions. |
| pyproject.toml | Bump project version to 0.2.1. |
| examples/quickstart.ipynb | Add consolidated quickstart notebook demonstrating the new API surface. |
| examples/nb_01_timeseries.ipynb | Remove old notebook (superseded by quickstart). |
| docs/usage.md | Update usage docs to metric-first and remove multivariate table section. |
| docs/overview.md | Update conceptual overview for descriptor/series split; remove table content; adjust geo section. |
| docs/index.md | Update docs landing page feature list for descriptor/units validation and removal of table. |
| docs/examples/timeseriestable.ipynb | Remove table example notebook. |
| docs/examples/timeseries.ipynb | Remove old time series example notebook. |
| docs/examples/index.md | Update examples index text after notebook removals. |
| docs/conf.py | Remove excluded internals that belonged to TimeSeriesTable. |
| docs/api.md | Remove API docs entry for TimeSeriesTable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Slim the public surface of
TimeSeriesandTimeSeriesDescriptorto pure time-series metadata:nameis now required on bothTimeSeriesandTimeSeriesDescriptor(str, no longerOptional[str]). The field already existed and was already calledname— this just makes it mandatory.labelsandlocationfromTimeSeries,TimeSeriesDescriptor, andTimeSeriesTable. Identity-shaped concepts (labels, tags) and spatial context belong to consumer layers (energydatamodel,energydb), not to time-series metadata itself.TimeSeriesTable— removed from__init__.py/__all__and its repr mixin. The class stays intimedatamodel/timeseriestable.pyas an internal utility; higher-level packages own the collection abstraction.units.py/validate_unit()— new module with a shared lazypint.UnitRegistryand avalidate_unit()helper. Covered bytests/test_units.py.nb_01_timeseries.ipynb,nb_02_timeseriestable.ipynb, and their docs counterparts replaced by a singleexamples/quickstart.ipynb.tests/test_timeseriestable.pyremoved._repr.py— ~95 lines lighter after dropping theTimeSeriesTablerepr mixin and location formatting.