Unexport numtype

[lbenet]

No description provided.

[lbenet]

In #496, numtype was introduced to have the same meaning than the previous usage we had for eltype, for the cases we required the internal element types on an Interval, so eltype(::Interval) behaves as a regular Real.

I used the same idea in JuliaDiff/TaylorSeries.jl#289 (see JuliaDiff/TaylorSeries.jl#224). Now, we get the following warning:

julia> using TaylorSeries

julia> using IntervalArithmetic
WARNING: using IntervalArithmetic.numtype in module TaylorSeries conflicts with an existing identifier.

despite the fact that TaylorSeries does not export it. This appears, e.g, in TaylorModels...

This PR proposes to not export numtype, which imposes that something that uses it has to qualify it.

cc @lucaferranti

[codecov-commenter]

Codecov Report

Merging #499 (8da6f89) into master (e94b3ae) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #499   +/-   ##
=======================================
  Coverage   91.09%   91.09%           
=======================================
  Files          25       25           
  Lines        1785     1785           
=======================================
  Hits         1626     1626           
  Misses        159      159           

Impacted Files	Coverage Δ
src/IntervalArithmetic.jl	100.00% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e94b3ae...8da6f89. Read the comment docs.

[lucaferranti]

I'm fine with not exporting it, The warning is pretty strange though, since it seems everything works

julia> using IntervalArithmetic

help?> numtype
search: numtype

  numtype(::Interval{T}) where {T<:Real} = T

  Returns the type of the bounds of the interval.

  Example
  –––––––––

  julia> numtype(1..2)
  Float64

julia> using TaylorSeries
WARNING: using IntervalArithmetic.numtype in module TaylorSeries conflicts with an existing identifier.

  julia> numtype(1..2)
  Float64

julia> methods(numtype)
# 2 methods for generic function "numtype":
[1] numtype(::Interval{T}) where T<:Real in IntervalArithmetic at /home/lferrant/.julia/packages/IntervalArithmetic/AAfrU/src/intervals/functions.jl:36
[2] numtype(x::IntervalBox{N, T}) where {N, T<:Real} in IntervalArithmetic at /home/lferrant/.julia/packages/IntervalArithmetic/AAfrU/src/multidim/intervalbox.jl:44

julia> numtype(1..2)
Float64

[schillic]

I can explain. The using .IntervalArithmetic here imports all exported functions from IntervalArithmetic, including numtype. By the time this code is executed, TaylorSeries has already defined its own numtype function here.
Now if IntervalArithmetic was an ordinary dependency, TaylorSeries could just do import IntervalArithmetic: numtype before defining its own methods. But the package is loaded via Requires. I am not aware of an easy way to deal with name clashes using Requires. Possible solutions:

1. Create a new auxiliary package that exports the function and then let both packages import it. That would also allow you to export the function in TaylorSeries.
2. Rename the function in TaylorSeries.
3. As you suggest here, do not export numtype in IntervalArithmetic (which I think is not too nice because it is a useful function, but I understand that the alternatives are also not ideal).

[lucaferranti]

A-haa, thank you Christian for the detective work! I didn't notice IntervalArithmetic.jl was an optional dependency, now it make sense! (I probably shouldn't review PRs before breakfast)

You mentioned numtype is not exported in TaylorSeries.jl, is it needed only internally, or can it be useful for "end-users"? If it's only for internal use, it could be renamed e.g. to _numtype to avoid the problem

[lbenet]

Thanks a lot @lucaferranti and @schillic for looking into this.

To answer Luca's question, numtype is not exported from TaylorSeries, but it can be used by the user as TaylorSeries.numtype, i.e., by qualifying the module. The same would happen if we rename it, though indeed that would not throw the warning.

The reason I used the same name is because the functions do really the same in both packages: for a "parametric numeric" (sub)type such as Interval{T}<:Real or Taylor1{T}<:Number, it extracts the element type T of the underlying components. The use case is because we sometimes need to know in advance the type of some resulting expression; in TS it allows to allocate the vectors of the output.

The subtlety becomes important in TaylorModels where each instance is used. (See e.g. here or here.) Since at the end we need to qualify the module that created the concrete method, I see no need to export it (solution 3).

[lucaferranti]

I see, I think it's ok to unexport it as done here. @lbenet as far as I'm concerned this can be merged

[lbenet]

Thanks a lot Luca and Christian for your comments. I'll bump a patch version and then merge.