Optionally treat NaNs as equal to each other in isapprox

[giordano]

@test_approx_eq has been deprecated by #19880, now the suggested way to compare numbers is @test isapprox(a ,b) (or some other fancy Unicode syntax). However, @test_approx_eq had a feature that isapprox is missing: @test_approx_eq NaN NaN didn't error. When performing tests, it makes sense to treat NaNs as equal (i.e., what isequal does): even if isnan is available, having isapprox dealing nicely with NaNs may be useful for cases like isapprox([1, NaN, Inf], [1, NaN, Inf]), where there is a mixture of numbers and not-numbers.

My suggestion: optionally have isapprox treat NaN values as equal to each other (maybe only in tests).

A bit of background: in a package of mines I have a test where I was using @test_approx_eq, relying on the fact that it treated NaNs as equal. When I moved to @test + isapprox I had to define a new method for a custom type in the testing suite and now I'm fine. With this ticket my aim is not to advocate for this feature, because I currently don't need it, but to suggest a possible improvement to isapprox that may be useful above all for testing.

[yuyichao]

This also affects Images.jl.

[JeffreySarnoff]

Make sense to me.

function isapprox(x::Number, y::Number; 
                  rtol::Real=rtoldefault(x,y), atol::Real=0, nans::Bool=false)
    x == y ||
    (isfinite(x) && isfinite(y) && abs(x-y) <= atol + rtol*max(abs(x), abs(y))) ||
    (nans && (isnan(x) & isnan(y)))
end

julia> anan = NaN
julia> isapprox(anan, anan)
false
julia> isapprox(anan, anan, nans=true)
true

[StefanKarpinski]

I guess we should really do this in this release since we're deprecating @test_approx_eq.

[StefanKarpinski]

What should we call this keyword? nans=true/false is a bit inscrutable but nanseq is kind of ugly.

[StefanKarpinski]

I guess nans=true/false is ok in the interpretation that nans refers to the comparison NaN ≈ NaN and the value indicates whether that comparison is true or false.

[JeffreySarnoff]

That was my intent. Also nanseq, reads like "nan sequence", so nans or eqnans unless equivnans.

[JeffreySarnoff]

I have prepared candidate changes, if you would like to take a look:
https://github.com/JeffreySarnoff/julia/blob/js/isapprox/base/floatfuncs.jl
https://github.com/JeffreySarnoff/julia/blob/js/isapprox/test/floatfuncs.jl
Or I could submit them as a PR.

[StefanKarpinski]

A PR would be lovely!

[JeffreySarnoff]

PR #20022

[tkelman]

Thanks @JeffreySarnoff !