Skip to content
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

(HashIndices([1, missing]) == HashIndices([1, missing])) == missing? #11

Closed
tkf opened this issue Feb 18, 2020 · 5 comments
Closed

(HashIndices([1, missing]) == HashIndices([1, missing])) == missing? #11

tkf opened this issue Feb 18, 2020 · 5 comments

Comments

@tkf
Copy link

tkf commented Feb 18, 2020

Writing JuliaLang/julia#34744 (comment), I started wonder if == on your set types (AbstractIndices) would be defined in terms of strong_equal(a, b) = isequal(a, b) && a == b as they are the mapping to itself? Also, wouldn't this solve the O(N^2) problem for defining == you mentioned?

@rapus95
Copy link

rapus95 commented Feb 18, 2020

what is the purpose of combining those with &&? On floats the former will be false sometimes while for missing the latter will be false sometimes.

@tkf
Copy link
Author

tkf commented Feb 19, 2020

This is due to two lines of thoughts:

  • We have HashIndices <: AbstractDictionary; i.e., isequal(idx[x], x) if idx isa HashIndices. So, if AbstractDictionary uses == on values, defining ==(::HashIndices, ::HashIndices) based on isequal(a, b) && a == b is a natural/automatic consequence from these semantics. That is to say, keys have to match w.r.t isequal and values have to match w.r.t ==. When keys are values, they have to match w.r.t isequal and ==.
  • Hash-based container needs an equality function eq such that eq(a, a) implies hash(a) == hash(b). However == does not support this. &&ing with isequal is one way to get this invariance.

@andyferris
Copy link
Owner

andyferris commented Jun 8, 2020

Yes, I'd like to implement == in a better way, I was thinking isequal on the keys and == on the values, and yes your point is well taken that this is what should happen on AbstractIndices.

@tkf I'm sorting through some ordering stuff at the moment - dId you have thoughts on HashIndices([1, 2]) == HashIndices([2, 1])? Here isequal is definitely false. Currently I'm leaning towards false for == and getitng people to use issetequal for set comparison.

@tkf
Copy link
Author

tkf commented Jun 9, 2020

dId you have thoughts on HashIndices([1, 2]) == HashIndices([2, 1])? Here isequal is definitely false.

You mean after #13 (so HashIndices would be order-dependent), right?

Yeah, if the elements of the container do not distinguish == and isequal (e.g., they are Ints) I think it makes sense for == and isequal to do the same thing. Letting people use issetequal to ignore ordering sounds good to me.

@andyferris
Copy link
Owner

Yeah, if the elements of the container do not distinguish == and isequal (e.g., they are Ints) I think it makes sense for == and isequal to do the same thing. Letting people use issetequal to ignore ordering sounds good to me.

In #13 I'm adding isdictequal for dictionary comparison for exactly the same reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants