Functions not compatible with all Base-compatible types

[mkborregaard]

We've tried replacing Base.maximum with NaNMath.maximum in Plots, as we very often deal with Vectors with NaNs in them. NaNs are e.g. used as placeholders for breaks in line segments. However, this causes the package to fail, as sometimes maximum will be called on something that is not a Vector{AbstractFloat}:

ERROR: MethodError: no method matching maximum(::Base.Generator{Array{String,1},Base.#length})
You may have intended to import Base.maximum
Closest candidates are:
  maximum(::Array{T<:AbstractFloat,1}) where T<:AbstractFloat at /Users/michael/.julia/v0.6/NaNMath/src/NaNMath.jl:85

AFAICS we cannot guarantee in advance what type gets passed to maximum. Is there a workaround for this?

[mlubin]

I'll likely accept a PR generalizing the implementations so long as they remain reasonably simple or copies of what is in Base with slight modification.

[mkborregaard]

Would it be out of the question to simply import the Base functions here (silencing the warnings)? In that case I should think all non-Vector{AbstractFloat} calls (which can't have NaNs) will just fall back on the Base methods. I may be missing some complexity here, of course.

[mlubin]

Yes, because if we do that then the implementations in NaNMath would globally replace the implementations in Base whenever the NaNMath package is loaded. NaNMath is meant to be opt-in only.

[mkborregaard]

Right :-/

[mkborregaard]

We're exploring a local solution along the lines of:

import NaNMath
_extrema(x...) = Base.extrema(x...)
_extrema(x::AbstractVector{<:AbstractFloat}) = NaNMath.extrema(x)

[BobPortmann]

Since the functions in NaNMath are not exported, it seems to me that they could follow this pattern as well, i.e. inside NaNMath add

mean(x...) = Base.mean(x...)

Also, it would be great to define the other forms of the function inside NaNMath. e. g.

mean(x::AbstractArray{:<AbstractFloat}, region) = ...

This way if someone replaces mean by nm.mean inside their code everything works.

[mlubin]

Well then sometimes the method ignores NaNs and sometimes it doesn't. I'd rather have a method error than a surprising behavior like that.

[BobPortmann]

I thought that only subtypes of AbstractFloat support NaN's and so that would never happen (if all code paths that take AbstractFloat are handled in NaNMath). If only Float64 and Float32 support NaN's then the functions in NaNMath could be defined only for them, other types would fallback to the Base code.

[mlubin]

I thought that only subtypes of AbstractFloat support NaN's and so that would never happen (if all code paths that take AbstractFloat are handled in NaNMath).

That's a reasonable argument. I would consider a PR.

[BobPortmann]

OK. I'll try to put something together in the next few weeks.

[mkborregaard]

I thought that only subtypes of AbstractFloat support NaN's and so that would never happen

Note that I had a PR implementing this based on exactly this assumption (https://github.com/mlubin/NaNMath.jl/pull/21), but it was closed because the assumption isn't strictly true. Tuples can hold NaN s as welll (see the discussion on the PR). I'd still welcome this change, though.