Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More generic support for interval types. #17

Open
haberdashPI opened this issue Jul 18, 2022 · 4 comments
Open

More generic support for interval types. #17

haberdashPI opened this issue Jul 18, 2022 · 4 comments
Assignees

Comments

@haberdashPI
Copy link
Member

haberdashPI commented Jul 18, 2022

@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
Copy link
Member Author

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

@ericphanson
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants