Approximate element pairing for sets #67
Conversation
|
Thanks for the PR @spavikevik. Sorry I wasn't able to reply to your issue yesterday. When I saw your message I was original thinking that we can changes things such that However, I do like the user-experience here of providing a default a |
|
@jatcwang I thought it's worth to mention that my colleague at work had a suggestion about using an algorithm such as Hungarian Matching to always find the best candidates for pairing but I think its asymptotic complexity would be too bad for this use case.. I do wonder if there's more edge cases here that we can catch using some heuristics, however. |
| false | ||
| } | ||
| val found = expWithIdx.find { case (e, idx) => | ||
| val res = itemDiffer.diff(a, e) |
There was a problem hiding this comment.
Hm I don't think we can take this out of the if statement otherwise we're going to be running diff for all elements in obtained against all elements in expected.
Perhaps we need to separate PairingFunction into two sub-traits: One that knows whether a pair matches without needing to run diff and one that doesn't? The former can be used for ordered collections such as Seq etc and the latter for unordered collections like Set?
There was a problem hiding this comment.
Perhaps something like this:
70c7bb1#diff-8183b3e34522b40214dc9c2bae969b6bdcc2d12f54eba714ba972ffd22d7cbd5R182-R198
Though I am not sure how you would feel about the boolean algebra one-liner there?
There was a problem hiding this comment.
@jatcwang hope you've been able to take a look at the changes.
Wishing you happy holidays and Happy New Year!
This tries to implement #66 in the following manner:
PairingFnwhich can be equals-based or differ-based:I tried to approach this in as common-sense way as possible and made some assumptions when implementing this such as using the approximate pairing function by default and specifying difference threshold to be 1, but I am open to discussing/adjusting them as needed.