search

beacon-biosignals / DataFrameIntervals.jl

Utilities for working with DataFrames of `Intervals.jl` or `TimeSpans.jl` objects.
MIT License
6 stars 1 forks source link

More generic support for interval types. #17

Open haberdashPI opened 2 years ago

haberdashPI commented 2 years ago

@ericphanson asked in #13 if there was some more generic way to support a variety of interval types.

I like the idea of using the Interval constructor. I think that would be the only thing necessary to make things work.

However, I think it is a little more involved to keep the exact behavior I'm aiming for, and as implemented currently. In each row I add a column that reports the intersection between the left and right dataframe's time span. This row is always the same type as left's timespan. For example, if you are working with dataframes that use TimeSpan the output also uses a TimeSpan.

There are a few ways forward to make that work:

  1. Define an IntervalsBase or DataFrameIntervalsBase package (or some other name): this essentially copies the interfaced implemented in each @require block, and then asks that each package implement that interface. There would be some work to identify the exact interface, and ideally keep the IntervalArray type private to DatatFrameIntervals. I think what this package would require would be a method to extract the endpoints and whether each is closed/open, and a method that looks a lot like backto (but with a better name).

  2. Support trait-bait dispatch in Intervals: the code internal to DataFrameIntervals to support multiple interval types is really just a way to work around the fact that the function find_intersections only works on Interval types. If we had some sort of trait-based dispatch on this function in Intervals, we wouldn't need anything special in DataFrameIntervals. This would probably involve defining some IntervalsBase package. Could be harder to achieve since it would require buy-in / discussion from more people (but that could also mean it would be a better interface!).

  3. Define AlignedSpans and TimeSpans as subtypes of Interval objects. This would eliminate everything but the NamedTuple support in DataFrameIntervals. @omus has already discussed doing this for TimeSpans here.

Having listed these out, 2 is really a further elaboration of 1, so the options are really 1 or 3 (with 1 possibly turning into 2 if it gets accepted as a part of Intervals.jl).

However, at the moment I lean towards option 3, and if that's where we want to end up, I would favor just using @requires until then.

haberdashPI commented 2 years ago

@ericphanson what're your thoughts on making AlignedSpans a subtype of AbstractInterval{T,Closed,Closed}?

ericphanson commented 2 years ago

I think that could work. Is there an interface specified for what behaviors it should have and what methods to implement? I'm not finding much in https://invenia.github.io/Intervals.jl/latest/#API-1

haberdashPI commented 2 years ago

Cool, I'll look into what it would take. I have pursued a lot of that repo already as part of the PR I wrote. (And maybe document the requirements while I'm at it!)

haberdashPI commented 1 year ago

I think it is going to be a somewhat long-term project to update Intervals to have a more generic interface, I have a draft I worked on for a bit here: https://github.com/invenia/Intervals.jl/pull/206.

I thought of a more general solution, and possibly more flexible one in #26