You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@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:
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).
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!).
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.
The text was updated successfully, but these errors were encountered:
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!)
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: invenia/Intervals.jl#206.
I thought of a more general solution, and possibly more flexible one in #26
@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 aTimeSpan
.There are a few ways forward to make that work:
Define an
IntervalsBase
orDataFrameIntervalsBase
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 theIntervalArray
type private toDatatFrameIntervals
. 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 likebackto
(but with a better name).Support trait-bait dispatch in
Intervals
: the code internal toDataFrameIntervals
to support multiple interval types is really just a way to work around the fact that the functionfind_intersections
only works onInterval
types. If we had some sort of trait-based dispatch on this function inIntervals
, we wouldn't need anything special inDataFrameIntervals
. This would probably involve defining someIntervalsBase
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!).Define AlignedSpans and TimeSpans as subtypes of
Interval
objects. This would eliminate everything but theNamedTuple
support inDataFrameIntervals
. @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
@require
s until then.The text was updated successfully, but these errors were encountered: