Support AlignedSpans

[haberdashPI]

This adds support for specifying intervals of time using AlignedSpans

[codecov]

Codecov Report

Merging #13 (879cab1) into main (49f71fa) will increase coverage by 0.32%.
The diff coverage is 93.75%.

❗ Current head 879cab1 differs from pull request most recent head 45b6047. Consider uploading reports for the commit 45b6047 to get more accurate results

@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
+ Coverage   89.50%   89.83%   +0.32%     
==========================================
  Files           1        1              
  Lines         162      177      +15     
==========================================
+ Hits          145      159      +14     
- Misses         17       18       +1     

Impacted Files	Coverage Δ
src/DataFrameIntervals.jl	89.83% <93.75%> (+0.32%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[ericphanson]

Is explicit support necessary? What do we need to do in AlignedSpans so it isn't?

AlignedSpans already support TimeSpans.istimespan, TimeSpans.start and TimeSpans.stop. We could add an Intervals constructor as well if that would help.

[haberdashPI]

Good question! I think, short term, that this PR is the easiest path forward. But I think making the support more generic is a good idea. I've opened #17.

[ericphanson]

We could add a constructor like

function Intervals.Interval{Nanosecond, Closed, Open}(span::AlignedSpan)
    L = time_from_index(span.sample_rate, span.first_index)
    R = time_from_index(span.sample_rate, span.last_index+1)
    return Interval{Nanosecond, Closed, Open}(L, R)
end

and even also

function Intervals.Interval{Nanosecond, Closed, Closed}(span::AlignedSpan)
    L = time_from_index(span.sample_rate, span.first_index)
    R = time_from_index(span.sample_rate, span.last_index)
    return Interval{Nanosecond, Closed, Closed}(L, R)
end

to TimeSpans.

To keep the exact behavior the same I also need a way to return the same interval type

Ah, that seems harder to do generically. I'm not sure if there's a reaosnable way to support it without the explicit support.

I am not a fan of Requires though; e.g. SciML style says

Requires.jl should be avoided at all costs

I have heard lots of folks have various issues with it, including load time regressions, and there are scary-looking unaddressed issues like JuliaPackaging/Requires.jl#83.

[haberdashPI]

I agree that @require is not ideal. I think how to make this more generic depends on whether we want to go with a IntervalsBase-like approach, an approach where we make everything an Interval, or something else (see #17).

[ericphanson]

Since AlignedSpan's roundtrips w/ TimeSpans, what about something like this?

using DataFrameIntervals
using TimeSpans
using DataFrameIntervals: Interval, IntervalArray, interval, backto
function DataFrameIntervals.interval(x::AlignedSpan)
    return interval(TimeSpan(x))
end
function DataFrameIntervals.backto(x::AlignedSpan, x_::Interval)
    return AlignedSpan(x.sample_rate, backto(TimeSpan(x), x_), RoundSpanDown)
end

function DataFrameIntervals.IntervalArray(x::AbstractVector{<:AlignedSpan})
    return IntervalArray(TimeSpan.(x))
end

[haberdashPI]

Ah! Yeah, that seems like a good interim approach! And would mean we don't have to wait until Intervals has some fully generic approach.

[palday]

I would suggest treating this approach as stale and if we want explicit support, using package extensions