Support AlignedSpans

[haberdashPI]

This adds support for specifying intervals of time using AlignedSpans

[codecov[bot]]

Codecov Report

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

:exclamation: 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%)	:arrow_up:

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

[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 https://github.com/JuliaPackaging/Requires.jl/issues/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