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

fix interval selection when keys are intervals #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aplavin
Copy link
Collaborator

@aplavin aplavin commented Jul 30, 2022

it wasn't possible to simply index by key if keys are intervals themlselves

@aplavin
Copy link
Collaborator Author

aplavin commented Jul 30, 2022

nightly failures are unrelated

@aplavin
Copy link
Collaborator Author

aplavin commented Aug 19, 2022

bump

@@ -12,6 +12,13 @@ findindex(int::Interval, r::AbstractVector) =
findindex(int::Interval, r::AbstractRange{T}) where {T<:Union{Number,Char}} =
findall(in(int), r)

# find interval in a vector of intervals: same as generic findindex in lookup.jl
function findindex(int::Interval, r::AbstractVector{<:Interval})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I suppose this is a bit ambiguous as you could also want to find the first occurrence in which int overlaps or intersects with one of the intervals in r. I suppose that's what the functional interface is for though? @mcabbott do you have a preference?

Copy link
Collaborator Author

@aplavin aplavin Sep 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My motivation was that keeping invariants goes a long way. And "calling with a value of type eltype(axiskeys(A)) returns the array value at that specific axiskey value" is a nice invariant to follow whenever possible. Overlaps or whatever else can be done with separate selectors, potentially as simple as !isdisjoint(int)/⊆(int)/....

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

Successfully merging this pull request may close these issues.

2 participants