JuliaGeometry / Quaternions.jl

A Julia implementation of quaternions
https://juliageometry.github.io/Quaternions.jl
MIT License
116 stars 37 forks source link

Add more methods to `Base.:(==)` #139

Closed hyrodium closed 9 months ago

hyrodium commented 9 months ago

Overview

Before this PR

julia> using Quaternions

julia> quat(1) == 1
true

julia> quat(1) == complex(1)
ERROR: promotion of types Quaternion{Int64} and Complex{Int64} failed to change any arguments
Stacktrace:
[...]

julia> quat(1,1,0,0) == complex(1,1)
ERROR: promotion of types Quaternion{Int64} and Complex{Int64} failed to change any arguments
Stacktrace:
[...]

After this PR

julia> using Quaternions

julia> quat(1) == 1
true

julia> quat(1) == complex(1)
true

julia> quat(1,1,0,0) == complex(1,1)
false

Why add methods?

Base has the following ==(::Real, ::Complex) and ==(::Complex, ::Real) definitions.

https://github.com/JuliaLang/julia/blob/e7345b89fd4eb15e8f395395701e19be705d7b06/base/complex.jl#L247-L249

Quaternions.jl should have methods similar to these, instead of == by type promotions.

How about compatibility with the Complex type?

There were the following choices for the design:

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2055de5) 100.00% compared to head (670aa2a) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #139 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 2 2 Lines 164 168 +4 ========================================= + Hits 164 168 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sethaxen commented 9 months ago

In my opinion, any code that mixes complex numbers and quaternions is doing the wrong thing. We're already explicit that there's no natural embedding of complex numbers in the quaternions, and I think it's perfectly fine if we don't have an equality method between quaternions and complex numbers at all, so that even pure real complex numbers and quaternions are unequal.

If every hypercomplex number type needed to add an equality method like this for every other hypercomplex number type, we would have combinatorial explosion and probably lots of unintended ambiguities.

hyrodium commented 9 months ago

Thank you for the comment! I agree with that and close this PR.