Add rich comparison for dict_iterator#2203
Conversation
| if size_func(zelf.len(), other.len()) { | ||
| return Ok(false); | ||
| } | ||
| let other_cp = other.copy(); |
There was a problem hiding this comment.
I guess this is not nessessary if we fix original code a bit to iterate smaller dict for instead of zelf.
Then remained size of the other side of looped one will tell it is bigger than smaller one or not.
There was a problem hiding this comment.
Thank you for your advice. I revised it to iterate a smaller one.
Modified inner_eq to inner_cmp so that dict_iterator can use and added rich comparison for dict_iterator.
| item: bool, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult<PyComparisonValue> { | ||
| if size_func(zelf.len(), other.len()) { |
There was a problem hiding this comment.
For eq and ne, this test leads the loop comparison.
But for other comparisons like lt, gt, the size comparison doesn't guarantee subset.
Assume a = {'a': 1, 'b' 2}; b = {'a': 1, 'b': 3, 'c': 4},
a.items() > b.items() is false but also b.items() > a.items() is false.
There was a problem hiding this comment.
Thank you for your advice! but don't the cpython also return false and false? I put it at first because it seems cpython check a length at first. link Should I change it as you said?
There was a problem hiding this comment.
Oh, I am sorry. I thought something totally wrong. Yes, it seems current behaviour is exactly as expected and also same as also what I described. Sorry for making confusion!
There was a problem hiding this comment.
No problem! I really want to thank you for your help.
2 participants
Modified inner_eq to inner_cmp so that dict_iterator can use and added rich comparison for dict_iterator.