equality and hash for terms and schemas#241
Conversation
Co-Authored-By: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
| length(first.schema) != length(second.schema) && return false | ||
| for key in keys(first) | ||
| !haskey(second, key) && return false | ||
| second[key] != first[key] && return false | ||
| end | ||
| true |
There was a problem hiding this comment.
Isn't this identical to first == second? If not, maybe worth a comment. (Note that this throws if the dict contains missing, maybe that's OK though.)
There was a problem hiding this comment.
Hmmm I think you're right. Probably fine to just check first.schema == second.schema since that's really what this check is about
| first === second && return true | ||
| first.schema === second.schema && return true |
There was a problem hiding this comment.
Note that this will be wrong if the dict contains missing (recursively). Can this happen?
There was a problem hiding this comment.
As a key or a value? Not possible either way at the moment (unless it's a pathological manuallly constructed instance)
There was a problem hiding this comment.
As a value I think, as for keys dicts use isequal. Note that this also applies if the value contains an object which contains a missing value (whatever the number of recursions).
| true | ||
| end | ||
|
|
||
| Base.hash(schema::Schema, h::UInt) = hash(schema.schema, h) |
There was a problem hiding this comment.
Sometimes (e.g. for arrays and tuples) we add an arbitrary constant to h (type-specific) to ensure that hash(schema) != hash(schema.schema). Not sure whether it's worth it here though.
There was a problem hiding this comment.
Yeah, I wondered about that too. Easy to do here, a bit tricker in the case of the terms (where the types might not be the asme for things that we want to be ==, e.g. different instances of a function term whre the anonymous function is different but the underlying expression is the same).
There was a problem hiding this comment.
And that can "corrupt" the type of containers because of the ahem zealous use of type parameters in this codebase
There was a problem hiding this comment.
You don't necessarily need to hash the type: you can just define a constant and use it for all types which compare ==.
3 participants
based on #174 (upstream user/repo seems to have been deleted) with a few tweaks