-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
== vs isequal #18
Comments
I agree that the above definitions would make auto-hash-equal'd structs behave like other containers. Compare (with current release of julia> using AutoHashEquals
julia> @auto_hash_equals struct Foo{T}
_::T
end
julia> ==(Foo(0/0), Foo(0/0))
true
julia> isequal(Foo(0/0), Foo(0/0))
true
julia> ==(Foo(missing), Foo(missing))
true
julia> isequal(Foo(missing), Foo(missing))
true to julia> ==((0/0,), (0/0,))
false
julia> isequal((0/0,), (0/0,))
true
julia> ==((missing,), (missing,))
missing
julia> isequal((missing,), (missing,))
true |
JuliaServices#18 Analogous to ```julia julia> ==((missing,), (missing,)) missing julia> isequal((missing,), (missing,)) true ```
My opinion is that this is likely a change that this package should and will make. Do you want to offer a PR? |
Sure, I can do that. Should it be opt-in and non-breaking, or just a breaking change? BTW: my reasoning for why this is the right choice is a tiny bit different than the original post, in that I don't think Array is the right comparison, but rather NamedTuple's. I think they are essentially the canonical "anonymous record type", using "record type" to refer to a struct that is defined directly by the data stored in its fields (in contrast to a struct used to implement another data structure, like the |
I'm curious why AutoHashEquals defines
==
in terms ofisequal
. In the README it says:but it seems like the following would make more sense:
This should still guarantee that
hash(a) == hash(b)
impliesisequal(a, b)
. Looking at the definitions inBase
forAbstractArray
it seems like this is correct:isequal(A, B)
appliesisequal
element-wise and==(A, B)
applies==
element-wise.Code for reference:
Base.(:==)
Base.isequal
The text was updated successfully, but these errors were encountered: