add interval part3#567
Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes the “interval” plumbing for time series identification by allowing callers to disambiguate time series UUID lookup using an interval keyword, which is particularly relevant for forecast-like series where multiple records can share the same name/resolution.
Changes:
- Add
interval::Union{Nothing, Dates.Period} = nothingkeyword argument toget_time_series_uuid. - Forward
intervalthrough toget_time_series_metadataduring UUID lookup.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name::AbstractString; | ||
| interval::Union{Nothing, Dates.Period} = nothing, | ||
| ) where {T <: TimeSeriesData} | ||
| metadata = get_time_series_metadata(T, component, name) | ||
| metadata = get_time_series_metadata(T, component, name; interval = interval) | ||
| return get_time_series_uuid(metadata) |
There was a problem hiding this comment.
New interval keyword support in get_time_series_uuid isn’t covered by tests. Please add a unit test exercising the disambiguation case (e.g., two forecasts with same type/name/resolution but different interval, and verify the correct UUID is returned when interval is provided, and that omission is ambiguous/throws if that’s the intended behavior).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #567 +/- ##
==========================================
+ Coverage 77.78% 78.87% +1.08%
==========================================
Files 75 75
Lines 6733 6640 -93
==========================================
Hits 5237 5237
+ Misses 1496 1403 -93
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This one change wasn't hit by the tests in PSI, it got triggered later