Stop using Base.@irrational

[hyrodium]

This macro should not be used outside of Base. See JuliaLang/julia#47103 and JuliaLang/julia#46531.

[devmotion]

I'm well aware of this issue (see, e.g., #12 (comment) and TuringLang/MCMCDiagnosticTools.jl#63 (comment)). The main/only point of this package is that already existing definitions of irrationals, floating around in the ecosystem, are defined in a common package so it's a lightweight dependency and thereby makes it less likely that two packages define the same constants. Most constants have been defined in StatsFuns for a long time before this package was created and have been used in particular in different stats packages; IIRC then at some point someone wanted to use them in SpecialFunctions which does not want to add a dependency on StatsFuns (and StatsFuns even depends on SpecialFunctions). Of course, it's only the second best solution but I think it's definitely better than every package trying to redefine irrational constants.

[hyrodium]

I see. Sorry for bothering you.

There's one more thing I'd like to ask you about. Is there any problems if we define the constants with IrrationalConstant{sym} <: AbstractIrrational and @irrationalconstant (a macro to define IrrationalConstant just like Base.@irrational)? I thought this solution can resolve the type piracy and does not produce any other problems.

[devmotion]

Yes, I assume the way to fix this are custom types that are not owned by base, as suggested in JuliaLang/julia#46531 (comment). I'm not sure how many methods in base are defined for Irrational explicitly - the annoying thing with custom types would be that we would miss functionality that does not support AbstractIrrational.

[devmotion]

But at least a quick search does not reveal anything: https://github.com/JuliaLang/julia/search?p=1&q=Irrational

[hyrodium]

I have not checked all of the code, but it seems we need to define these methods at least.

show(io::IO, x::IrrationalConstant{sym}) where {sym} = print(io, sym)

function show(io::IO, ::MIME"text/plain", x::IrrationalConstant{sym}) where {sym}
    if get(io, :compact, false)::Bool
        print(io, sym)
    else
        print(io, sym, " = ", string(float(x))[1:min(end,15)], "...")
    end
end

==(::IrrationalConstant{s}, ::IrrationalConstant{s}) where {s} = true
<(::IrrationalConstant{s}, ::IrrationalConstant{s}) where {s} = false
<=(::IrrationalConstant{s}, ::IrrationalConstant{s}) where {s} = true
hash(x::IrrationalConstant, h::UInt) = 3*objectid(x) - h  # I'm not much familiar with `hash`. just copied from Base.

[devmotion]

I think that we would want to avoid the design of Irrational since that leads to exactly the same problems if some tries to extend our irrational type in a package. Basing irrationals on a type parameter seems flawed. Instead we would want separate singleton types for each irrational constants, as proposed in the issue linked above.

If you don't beat me to it, I might have time to look into it later this evening.

[devmotion]

Fixed by #19, will be released as IrrationalConstants 0.2.