beacon-biosignals / TimeSpans.jl

A Julia package that provides a `TimeSpan` type for representing a continuous span between two points in time.
Other
6 stars 2 forks source link

Add method for `Base.in` #9

Closed ericphanson closed 3 years ago

ericphanson commented 3 years ago

for the concrete TimeSpan object (not necessarily the generic interface, which I couldn't do without piracy). This allows things like:

julia> using TimeSpans, AxisKeys

julia> arr = KeyedArray(rand(20,1000), channel=1:20, time=Nanosecond.(Second.(1:1000)))
2-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   channel ∈ 20-element UnitRange{Int64}
→   time ∈ 1000-element Vector{Nanosecond}
And data, 20×1000 Matrix{Float64}:
        Nanosecond(1000000000)    Nanosecond(2000000000)   …   Nanosecond(1000000000000) 
  (1)   0.601813                  0.11964                      0.683801
    ⋮                                                      ⋱   ⋮
 (19)   0.101305                  0.818797                     0.05417
 (20)   0.453637                  0.394128                     0.41232

julia> arr(time=in(TimeSpan(Minute(1), Minute(2))))
2-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   channel ∈ 20-element UnitRange{Int64}
→   time ∈ 61-element view(::Vector{Nanosecond},...)
And data, 20×61 view(::Matrix{Float64}, :, [60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120]) with eltype Float64:
        Nanosecond(60000000000)    Nanosecond(61000000000)   …   Nanosecond(120000000000) 
  (1)   0.460153                   0.486687                      0.368666
    ⋮                                                        ⋱  
 (19)   0.469283                   0.921581                      0.298631
 (20)   0.0784988                  0.225722                      0.899711
jrevels commented 3 years ago

Hmm this is already available fully generically via the invocation TimeSpans.contains(span, TimeSpan(period)). I'd be worried about folks using this version of it in generic code, where it might break unexpectedly for non-TimeSpan timespans.

Though it does seem natural that this would be defined so maybe it's fine. Can also consider the comparison with Intervals.jl which should replace this package eventually (ref #2)

ericphanson commented 3 years ago

Hmm this is already available fully generically via the invocation TimeSpans.contains(span, TimeSpan(period)).

This doesn't work quite the same way:

julia> using TimeSpans

julia> span = TimeSpan(Second(1), Second(10))
TimeSpan(00:00:01.000000000, 00:00:10.000000000)

julia> period = Second(10)
10 seconds

julia> TimeSpans.contains(span, TimeSpan(period))
false

whereas for in it would be true.

Though it does seem natural that this would be defined so maybe it's fine.

Right, my thinking was other implementors of the TimeSpan interface could also implement in (and, I suppose, the findall workaround).

Can also consider the comparison with Intervals.jl which should replace this package eventually (ref #2)

Intervals.jl does implement in, although not the findall workaround, and I was relieved to note that promotion/conversion works correctly, e.g.

julia> using Intervals, AxisKeys, Dates

julia> arr = KeyedArray(rand(10), time = Nanosecond.(Second.(1:10)))
1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   time ∈ 10-element Vector{Nanosecond}
And data, 10-element Vector{Float64}:
  Nanosecond(1000000000)    0.5958239851664413
  Nanosecond(2000000000)    0.1600016244948943
  Nanosecond(3000000000)    0.5752673963608166
  Nanosecond(4000000000)    0.6781712575391108
  Nanosecond(5000000000)    0.016013494198902745
  Nanosecond(6000000000)    0.4658518273829213
  Nanosecond(7000000000)    0.9710087001236398
  Nanosecond(8000000000)    0.24007706917723137
  Nanosecond(9000000000)    0.2705505534526862
  Nanosecond(10000000000)   0.7206844164927835

julia> arr(time=t -> t in Intervals.Interval(Second(5), Second(10))) # work around `findall` being broken with curried `in`
1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
↓   time ∈ 6-element view(::Vector{Nanosecond},...)
And data, 6-element view(::Vector{Float64}, [5, 6, 7, 8, 9, 10]) with eltype Float64:
  Nanosecond(5000000000)    0.016013494198902745
  Nanosecond(6000000000)    0.4658518273829213
  Nanosecond(7000000000)    0.9710087001236398
  Nanosecond(8000000000)    0.24007706917723137
  Nanosecond(9000000000)    0.2705505534526862
  Nanosecond(10000000000)   0.7206844164927835

meaning that we can specify the Interval in seconds even though the axis is in nanoseconds.

ericphanson commented 3 years ago

I think in should not include the endpoint as I've done here though, since it's supposed to be exclusive! whoops. I'll fix that when I get a chance.

jrevels commented 3 years ago

Hmm this is already available fully generically via the invocation TimeSpans.contains(span, TimeSpan(period)). I'd be worried about folks using this version of it in generic code, where it might break unexpectedly for non-TimeSpan timespans.

I generally still feel ^this^ way about this but I think that concern is far outweighed by the usefulness of enabling TimeSpan to interop with AxisKeys in the aforementioned way (and other similar interop that might be enabled with other packages)

hopefully I can have my cake and eat it too once we deprecate this in favor of Intervals.jl