Intervals.jl
Intervals.jl copied to clipboard
Deprecate `first`/`last` in favor of `lower`/`upper`
It can be rather annoying to read code with a Vector{Interval}
as you can end up with things like: first(first(v))
which would be more readable as lower(first(v))
I'll also note this is the terminology used by PostgreSQL's range functions: https://www.postgresql.org/docs/9.3/functions-range.html
sure, seems fine to deprecatefirst
,last
; they are somewhat associated with iteration too, i.e. it's usual to expect first(x) == x[1]
, last(x) == x[end]
which of course is not true of non-iterable intervals.
in fact, the docstring of first
even says
help?> Base.first
first(coll)
Get the first element of an iterable collection.
lower
and upper
seem as good a name as anything else.
a more verbose option could be lowerbound
, upperbound
, but i'm unsure if that's necessary
arguably it is more consistent with lowercase
(which kindly did not take the lower
name for itself)
Also, lower
may be read as a verb (when it is intended as a noun), which favours lowerbound
/upperbound
.
Another alternative would be to overload minimum
and maximum
. i'm not sure this has the same association with iteration as first
/last
from a user perspective (although i think it does fallback to using iteration under the hood). I think it's just related to the minimum/maximum in a collection (and we do define in
). Although, if using minumum
/maximum
, then i think we'd want to respect the inclusivity
such that we always had e.g. minimum(x) in x
(this is not true for the current first
/last
). So perhaps minimum
/maximum
should be the inclusivity-respecting version of lower[bound]
/upper[bound]
... but that's now off-topic.
Although, if using
minumum
/maximum
, then i think we'd want to respect theinclusivity
such that we always had e.g.minimum(x) in x
(this is not true for the currentfirst
/last
). So perhapsminimum
/maximum
should be the inclusivity-respecting version oflower[bound]
/upper[bound]
... but that's now off-topic.
To stray further...the infimum
and supremum
could/would supplant first
and last
if we wanted to keep this functionality but use more mathematically appropriate terms.
-
infimum
andsupremum
names match IntervalSets.jl, DomainSets.jl, and TensorKit.jl (i think these all have pretty consistent semantics even though they don't extend a common function, and using those names here in Intervals.jl would also be consistent) -
inf
andsup
names matches InveralArithmetic.jl
--
- Pro:
infimum
is shorter thanlowerbound
- Con: it's a scary Latin word, I can never spell it right, and i always have to say in my head "sUP is the UPperbound, so the other one must be the lowerbound"
Also I would like my bikeshed to be blue please.
thinking about this.
For closed intervals,
maximum
and minimum
are correct, we are finding the largest and smallest elements in the set.
For open intervals, it is not technically true since the bounds are not included in the set.
For both open and closed infimum
and and supremum
would be correct.
if we wanted to get particular we could well define maximum(x::Interval{<:Any, Open) = supremum(x) - eps(eltype(x))
and similar for minumum
. (since DateTimes
etc all define eps
, though integers don't).
I would also say (that for closed intervals at least) first
and last
are not wrong.
An interval is an intrinstically ordered set, we know that because if you pu things in backwards it puts them in the right order.
And while it is not iterarable, we do know the first and last element that would be included, were it too be iterable -- it is a noniterable range.
I don't find first(first(::Vector{Interval}))
to be confusing.
As a new Intervals.jl
user, I also came to comment on this. I don't like first
and last
; start
/finish
might be an alternative not mentioned. One could use attributes? .begin
and .end
? Overall, I'm not afraid of long names, lowerbound
and upperbound
are quite clear and helpful.
Overall I think i am in favour of lowerbound
/upperbound
. They're accurate and non-ambiguous terms (and English not Latin).
Optionally, in future, we we could always add infimum
/supremum
too (and that'd be non-breaking, e.g. const infimum = lowerbound
) if we want to match any of IntervalSets.jl, DomainSets.jl, and TensorKit.jl.
One other thing to mention here is there is also a type associated with the lower/upper endpoint which represents the type of bound used (closed/open/unbounded). The terms lowerbound
/upperbound
may lead to some confusion as these types use the supertype: Bound
.
My own two cents:
lowerbound
/ upperbound
seem like the most obvious, but also think infimum
/ supremum
are fine (but easier to misspell).
I think the relationship to Bound
is actually consistent: both of them refer to the same thing. One is about whether the bound is closed or not and one is about if it's the upper or the lower bound. You could change it to Bounding
or Boundedness
to further clarify, but that doesn't seem necessary.
I think the overloading of Base.first
and Base.last
is bad enough to consider making this change soon. Is there any reason I should not make a PR for this?
Having taken a shot at this, I find that using lowerbound
and upperbound
is quite disruptive.
@omus concern re Bound
is more of an issue than I initially thought, at least if one wants the internal source code of this repo to not be confusing. The issue is as follows:
- If you rename
first
tolowerbound
, then - we should probably rename
LeftEndpoint
toLowerBound
(to avoid mixing terminology within the source code). - If we do that, then we should really rename
Endpoint
toBound
. - And that means we should really rename what was
Bound
toBoundedness
.
This also means the change would be breaking, without any space for deprecation, since Bound
is exported; it also looks like LeftEndpoint
etc... is assumed to be used externally (though not exported, there are deprecations).
So not only does it require a lot of carefully ordered find and replace calls within Intervals.jl, it would probably require the same or similar amount of effort for any callers of Intervals.jl
.
What if Intervals.jl
were to use left_endpoint
and right_endpoint
(or left
and right
), keeping the terminology used within the repo?
There's also start
and stop
(Which TimeSpans
uses).
Had an idea for how to make this a little disruptive than I initially thought, and have given that a try in #198.