Renaming of functions

[OlivierHnt]

IntervalBox is a wrapper that allows defining (unambiguously) set operations (union, intersect, isempty, etc.) in the sense of interval arithmetic.

This PR removes the IntervalBox struct, in favour of just using AbstractVector{<:Interval}.
Note that set operations (union, intersect, isempty, etc.) are now confusing since they have a completely different meaning for Interval and AbstractVector; thus this PR also renames such operations following the standard.

Name changes:

• in -> ismember
• isthin -> issingleton (aesthetic change to match the standard)
• isdisjoint -> disjoint
• union completely removed, it was an alias of hull
• hull -> convexhull (aesthetic change to match the standard)
• intersect -> intersection
• issubset -> subset
• isstrictsubset -> strictsubset
• isweaklyless -> less (aesthetic change to match the standard)
• setdiff -> setdiffinterval
• isempty -> isemptyinterval
• isentire -> isentireinterval (aesthetic change for consistency)
• ≛ (\starequal<tab>) -> ≐ (\dotequal<tab>, I changed the star for a dot, since in the standard I saw symbols with dots..) and is now in the Symbols module, defined to be the unicode alias of a new function equal

Of course disjoint, intersection, subset, less and equal are semantically very close to the Base functions is disjoint, intersect, issubset, isless and isequal respectively. Nevertheless, I think if we document this properly (using many admonitions 🙂) this should be find.

EDIT: see #572 (comment) for the current name changes.

I did not define all these functions for AbstractVector. Only bisect, mince and setdiffinterval are defined since the others can be defined in one line, e.g. intersection(x::AbstractVector) = intersection.(x). Yet, should we define such functions? One benefit could be that it allows for writing code that works seamlessly for an Interval or a multidimensional interval.

cc-ed @Kolaru

P.S.: unrelated to the rest, but while I was renaming things, I also changed midpoint_radius to midradius for consistency, since midpoint is the mid function.

[OlivierHnt]

Two comments:

1. it turns out that InteractiveUtils exports the name less as well. So we need to rename at least that one function.

2. PR Remove pointwise boolean functions #571 removed comparisons (i.e. "pointwise boolean functions" such as <, etc.) because these operations are notorious for silent failures. For instance, someone may not expect 0..2 < 1..3 to be true. In particular, the operation == now returns an error message inviting the user to use ≛ (renamed equal, or ≐ in the present PR).
   Yet, ==(::Interval, Interval) is not ambiguous in any way: the intervals are equal if they are identical.
   In fact, there is still ===, from Core, that is defined (and must be kept). In this regard, it is a bit inconsistent to prevent == from working.
   Was there any reasons, other than the one mentioned above, for introducing ≛? If not, I think it makes sense to keep == to check equality between intervals.

[Kolaru]

The typical corner case that requires to disallow == is again because of the drop-in replacement for floats. For example, the following would return wrong result when extended to interval if == is allowed`:

function diffexp(x, y)
    if x == y
        return 1
    else
        return exp(x - y)
    end
end

Keeping === on the other hand is fine, it is not a mathematical operator to begin with.

[OlivierHnt]

Yep ok that's fair; let's have as many alarm bells as possible!
So we could also take this PR as an opportunity to close #219 and #419, cf. this comment.

[OlivierHnt]

@Kolaru have you any feedback on this PR?

[codecov-commenter]

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (4757c9e) 85.91% compared to head (26e221e) 85.74%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #572      +/-   ##
==========================================
- Coverage   85.91%   85.74%   -0.18%     
==========================================
  Files          29       29              
  Lines        1640     1648       +8     
==========================================
+ Hits         1409     1413       +4     
- Misses        231      235       +4     

Files	Coverage Δ
src/IntervalArithmetic.jl	100.00% <100.00%> (ø)
src/bisect.jl	100.00% <100.00%> (ø)
src/decorations/decorations.jl	85.71% <ø> (+10.71%)	⬆️
src/decorations/functions.jl	81.28% <100.00%> (ø)
src/decorations/intervals.jl	85.18% <100.00%> (ø)
src/display.jl	75.00% <100.00%> (ø)
src/intervals/arithmetic/absmax.jl	85.71% <100.00%> (ø)
src/intervals/arithmetic/basic.jl	97.85% <100.00%> (ø)
src/intervals/arithmetic/hyperbolic.jl	100.00% <100.00%> (ø)
src/intervals/arithmetic/integer.jl	100.00% <100.00%> (ø)
... and 15 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Kolaru]

The renaming need to be discussed separately from the removal of IntervalBox, they are orthogonal issues. During the last JuliaInterval call, it appears the name changes were very controversial. I don't quite see the need to avoid extending base methods on interval. An interval is both an arithmetic object[*] and a set, so implementing both methods seem fine.

I therefore suggest to open another PR with only the IntervalBox removed, and use this one to discuss the renaming.

[*] A concept I just invented to avoid saying it is a number ^^'

[OlivierHnt]

Following the triage call, this PR is only about changing the function names to resolve the confusion induced by overloading Base functions (such as in, or intersect) if IntervalBox is removed, cf. PR #582.

Here are the only function with name changes:

• in -> ismember
• isdisjoint -> isdisjointinterval
• union completely removed (it was a mere alias of hull)
• intersect -> intersectinterval
• issubset -> isweaksubset
• isempty -> isemptyinterval
• isentire -> isentireinterval
• ≛ (\starequal<tab>) -> unicode alias of isequalinterval (important to have a non-unicode version of a function name)

Should fix #409. We could also address the bug reported in #219 and #419 in this PR, what do you think?

[Kolaru]

We discussed it in the call today and realized the renaming has a deeper meaning.

Namely, the idea is to "unsupport" intervals as 1 element arrays. This is the default for Real in Base, as all numbers are iterable. For example, intersection of Real is valid

julia> intersect(2, 3)
Int64[]

julia> intersect(34, 34.0)
1-element Vector{Float64}:
 34.0

This is a problem for us because intersection of intervals, and intersection of 1 element arrays containing an interval are different.

By disallowing both, we avoid silent ambiguity that can lead to incorrect behaviors. Thus I am in favor of the renaming.

What the name should be is not so clear. I am fine with the name proposed. A possible alternative (possibly more elegant) would be to add a singleton to tell how the set operation should be understood.

For example

struct IntervalAsSet end

intersect(::IntervalAsSet, x::Interval, y::Interval)

That way we keep Base names, while avoiding ambiguity.

@lucaferranti @dpsanders @lbenet

[OlivierHnt]

I have added explicit error messages for ==, <, isdisjoint, issubset, issetequal, in, isempty, isfinite, isnan, isinteger. These are also triggered implicitly when calling ≤, ≥, isequal, isless, >, isinf.

We can do the same for set operations to disambiguate, namely intersect(!), union(!), setdiff(!) (I may be missing some). Yet I wonder how much this will interfere with other parts of Base, which may need these functions to be defined and behave a specific way for a Real.

[OlivierHnt]

In the last commit I removed isstrictsubset/isstrictsupset since these are equivalent to isinterior (except technically for empty intervals but I think the isinterior one is better).
I felt that the new naming were a little too random. Hence for consistency I also did the renaming:

• isweaksubset -> isweakinterior (isweaksupset is removed)
• isinterior ->istrictinterior
• precedes -> weakprecedes

[Kolaru]

Only one small think: wouldn't it be more consistent to have all Base set methods become __interval ? That would just mean

• in -> ininterval (instead of ismember)
• issubset -> issubsetinterval (insteand of isweaksubset)

I don't have a definitive opinion on that. I think either way, this PR can be merged now.

[OlivierHnt]

So here are the name changes:

• in -> in_interval
• isdisjoint -> isdisjoint_interval
• intersect -> intersect_interval
• isempty -> isempty_interval
• isentire -> isentire_interval
• ≛ -> isequal_interval (with ≛ as an alias)
• issubset -> issubset_interval
• isinterior -> isstrictsubset_interval
• precedes -> precedes_interval
• strictprecedes -> strictprecedes_interval
• setdiffinterval -> setdiff_interval
  Also, union is removed, allowing us to define in the future a union_interval function to take the disjoint union of intervals.

Note 1: array operations intersect, setdiff, etc., do not raise an error; they behave as they do for a Real.

Note 2: I did not rename emptyinterval to empty_interval (nor entireinterval to entire_interval).

Note 3: precedes and strictprecedes do not have conflicts with Base functions so the renaming to precedes_interval and strictprecedes_interval is optional. So in principle we could keep them as precedes and strictprecedes, the same way we keep hull, instead of defining hull_interval. What are your thoughts on this?

Do not hesitate if you have more suggestions.

cc @Kolaru, @lbenet