Open timholy opened 6 months ago
The proposed change is separable from the AbstractArray
matter.
The documented definition of eltype
is as follows.
Determine the type of the elements generated by iterating a collection of the given type.
As long as iterate
and getindex
return types are consistent with eltype
then I think we can subtype the AbstractArray
interface.
Using getindex
with non-integer indices has been deprecated for about six years in this package.
julia> itp[1.5]
┌ Warning: `getindex(itp::AbstractInterpolation{T, N}, i::Vararg{Number, N}) where {T, N}` is deprecated, use `itp(i...)` instead.
│ caller = top-level scope at REPL[13]:1
└ @ Core REPL[13]:1
1.5
We should remove methods of getindex
for non-integers now. Correspondingly, we may also want to change the eltype
to narrower definition.
I would not expect eltype
to provide information about the return type from the function call syntax. I would use Base.return_types
for that or in Julia 1.11, Base.infer_return_type
.
Interesting. There's a lot I like about that. But for a possible counterview: a function is not iterable. Is an interpolant any different from a function?
I've seen cases that make me think that we may need to redesign how construction and position-evaluation compute types. To be clear, I suspect that I am responsible for the current situation, so any weaknesses are my fault. This interacts quite closely with #242 and perhaps it would be best to address both together. See also #359.
There is some ambiguity about what "element type" means for a container like an AbstractInterpolation. At one point, I think the clearest statement came from Jeff Bezanson, which is that the eltype should be the type of object returned when evaluated at integer positions, much as how
getindex
works onAbstractArrays
. However, evaluating at other positions (or with positions encoded by non-integer types) may return objects of different type, because the mathematics performed may be different. One strong possibility is that if we implement #242, then we should makeeltype(::AbstractInterpolation)
aMethodError
.I've put together a short, simple test that illustrates how I think it should work. Currently, nearly half of these fail.
Take-aways
We shouldn't use
promote
orpromote_type
. Fundamentally, promotion is about storage ("how do I create a container that can hold both of these objects?"), and that's different from computing the type of object returned by, e.g.,(1-f)*a + f*b
.Downstream users, however, may receive new bugs from the lack of assumed real-arithmetic "widening". That is, currently we have
because we effectively assume that the created object will be evaluated at
Float64
-encoded positions. If we change that toInt
, as I'm proposing here (because the coefficients areInt
), I can imagine that will break a lot of downstream code that relies oneltype(itp)
returning something that can accomodate bothInt
andAbstractFloat
evaluation locations. Makingeltype
throw aMethodError
would be one way of preventing more subtle bugs because of this change.