bug: overly permissive methods causing ambiguity

[nsajko]

Some of the methods defined in this package, such as the two-argument methods for the arithmetic functions, such as +, -, *, / have type signatures that are invalid because of being overly permissive. The issue is that ambiguity arises as soon as another package decides to do the same thing. Example:

julia> using DynamicQuantities

julia> q = 0.2u"m/s"
0.2 m s⁻¹

julia> struct Z <: Number end

julia> Base.:(*)(::Number, z::Z) = z

julia> q * Z()
ERROR: MethodError: *(::Quantity{Float64, Dimensions{FixedRational{Int32, 25200}}}, ::Z) is ambiguous.

Candidates:
  *(l::AbstractQuantity, r::Number)
    @ DynamicQuantities ~/tmp/jl/DynamicQuantities.jl/src/math.jl:19
  *(::Number, z::Z)
    @ Main REPL[4]:1

Possible fix, define
  *(::AbstractQuantity, ::Z)

Stacktrace:
 [1] top-level scope
   @ REPL[5]:1

The solution is to set a more strict constraint for the "other type" in the type signature. Number is too permissive, as (some of) the quantity types defined here also subtype Number.

Some of the arithmetic methods defined in this package, those defined for AbstractGenericQuantity, do not constrain the "other type" at all!

[MilesCranmer]

Yes this is indeed tricky. What should we use for the other type?

[nsajko]

I believe the only way is to hardcode lots of methods with specific "other types".

For example, AbstractQuantity has Number as it's direct supertype, so the "other type" in the method definition has to be a strict subtype of Number, such as Real or Complex. AbstractRealQuantity has Real as it's direct supertype, so Real is overly permissive as the "other type", etc.

[MilesCranmer]

That makes a lot of sense to me. I guess AbstractRealQuantity would be things like AbstractFloat?

[nsajko]

Forgot to mention: adding these overly permissive methods also invalidates a chunk of the sysimage. Data (for nightly Julia, without loading the REPL or any other package):

• DynamicQuantities.json

"method": "*(l::DynamicQuantities.AbstractGenericQuantity, r) @ DynamicQuantities ~/.julia/packages/DynamicQuantities/GdHhF/src/math.jl:19"

"method": "+(l, r::DynamicQuantities.AbstractGenericQuantity) @ DynamicQuantities ~/.julia/packages/DynamicQuantities/GdHhF/src/math.jl:72"

"method": "-(l, r::DynamicQuantities.AbstractGenericQuantity) @ DynamicQuantities ~/.julia/packages/DynamicQuantities/GdHhF/src/math.jl:72"

[MilesCranmer]

Is there a non-breaking fix we could apply for these ones? i.e., is it similar to the >= fix in that some of these are redundant?

[nsajko]

I don't think so.