Originally created by @jw-y on GitHub (Apr 4, 2024).
local com = (a, b) -> a >= b
local a = List(0, 0, 0, 0, 1, 1, 1, 1, 1, 1)
b = a.sortWith(com)
expected output:
b = List(1, 1, 1, 1, 1, 1, 0, 0, 0, 0)
got:
b = List(1, 1, 1, 1, 0, 0, 0, 0, 1, 1)
Originally created by @jw-y on GitHub (Apr 4, 2024).
``` pkl
local com = (a, b) -> a >= b
local a = List(0, 0, 0, 0, 1, 1, 1, 1, 1, 1)
b = a.sortWith(com)
```
expected output:
```
b = List(1, 1, 1, 1, 1, 1, 0, 0, 0, 0)
```
got:
```
b = List(1, 1, 1, 1, 0, 0, 0, 0, 1, 1)
```
I don't think this is a bug.
The documentation of List.sortWith says:
comparator should return true if its first argument comes before its second argument in the sort order, and false otherwise.
So it should be local com = (a, b) -> a > b instead of local com = (a, b) -> a >= b.
I checked Swift's and Scala's sortWith functions, and they have the same requirement. Swift additionally states that the comparator function must be irreflexive.
@odenix commented on GitHub (Apr 4, 2024):
I don't think this is a bug.
The documentation of `List.sortWith` says:
> comparator should return true if its first argument comes before its second argument in the sort order, and false otherwise.
So it should be `local com = (a, b) -> a > b` instead of `local com = (a, b) -> a >= b`.
I checked Swift's and Scala's `sortWith` functions, and they have the same requirement. Swift additionally states that the comparator function must be *irreflexive*.
@jw-y commented on GitHub (Apr 4, 2024):
Either case, I don't think this is any way intended.
FYI,
found the location of the bug.
https://github.com/apple/pkl/blob/58ed8242af664ecf156ad1ea72a79214c794b4bc/pkl-core/src/main/java/org/pkl/core/stdlib/base/MergeSort.java#L61
first `mid` should be `mid-1`
That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.
@odenix commented on GitHub (Apr 4, 2024):
That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Originally created by @jw-y on GitHub (Apr 4, 2024).
expected output:
got:
@holzensp commented on GitHub (Apr 4, 2024):
This is wild! I managed to reproduce. Thanks for the bug report.
@odenix commented on GitHub (Apr 4, 2024):
I don't think this is a bug.
The documentation of
List.sortWithsays:So it should be
local com = (a, b) -> a > binstead oflocal com = (a, b) -> a >= b.I checked Swift's and Scala's
sortWithfunctions, and they have the same requirement. Swift additionally states that the comparator function must be irreflexive.@jw-y commented on GitHub (Apr 4, 2024):
Either case, I don't think this is any way intended.
FYI,
found the location of the bug.
https://github.com/apple/pkl/blob/58ed8242af664ecf156ad1ea72a79214c794b4bc/pkl-core/src/main/java/org/pkl/core/stdlib/base/MergeSort.java#L61
first
midshould bemid-1@odenix commented on GitHub (Apr 4, 2024):
That indeed looks like a bug. I guess a reflexive comparator function should only give you an unstable/inefficient sort, not a wrong sort.