Skip to content

Commit

Permalink
Fixed issue with sort-* fns returning an error on empty seqs. (#717)
Browse files Browse the repository at this point in the history
Hi,

could you please review patch to fix an issue with `sort-*` erroring out
on an empty seq. Fixes #716.

It now returns an empty list in accordance with clojure

https://github.com/clojure/clojurescript/blob/e7cdc70d0371a26e07e394ea9cd72d5c43e5e363/src/main/cljs/cljs/core.cljs#L2467-L2479

I've included tests for the same.

Thanks

Co-authored-by: ikappaki <[email protected]>
  • Loading branch information
ikappaki and ikappaki authored Dec 4, 2023
1 parent 5373531 commit 44760b6
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Fix `sort` support for maps and boolean comparator fns (#711).
* Fix `(is (= exp act))` should only evaluate its args once on failure (#712).
* Fix issue with `with` failing with a traceback error when an exception is thrown (#714).
* Fix issue with `sort-*` family of funtions returning an error on an empty seq (#716).

## [v0.1.0a2]
### Added
Expand Down
84 changes: 46 additions & 38 deletions src/basilisp/lang/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -1399,35 +1399,39 @@ def sort(coll, f=compare) -> Optional[ISeq]:
using f or use the `compare` fn if not.
The comparator fn can be either a boolean or 3-way comparison fn."""
if isinstance(coll, IPersistentMap):
coll = lseq.to_seq(coll)
seq = lseq.to_seq(coll)
if seq:
if isinstance(coll, IPersistentMap):
coll = seq

comparator = _fn_to_comparator(f)
comparator = _fn_to_comparator(f)

class key:
__slots__ = ("obj",)
class key:
__slots__ = ("obj",)

def __init__(self, obj):
self.obj = obj
def __init__(self, obj):
self.obj = obj

def __lt__(self, other):
return comparator(self.obj, other.obj) < 0
def __lt__(self, other):
return comparator(self.obj, other.obj) < 0

def __gt__(self, other):
return comparator(self.obj, other.obj) > 0
def __gt__(self, other):
return comparator(self.obj, other.obj) > 0

def __eq__(self, other):
return comparator(self.obj, other.obj) == 0
def __eq__(self, other):
return comparator(self.obj, other.obj) == 0

def __le__(self, other):
return comparator(self.obj, other.obj) <= 0
def __le__(self, other):
return comparator(self.obj, other.obj) <= 0

def __ge__(self, other):
return comparator(self.obj, other.obj) >= 0
def __ge__(self, other):
return comparator(self.obj, other.obj) >= 0

__hash__ = None # type: ignore
__hash__ = None # type: ignore

return lseq.sequence(sorted(coll, key=key))
return lseq.sequence(sorted(coll, key=key))
else:
return llist.EMPTY


def sort_by(keyfn, coll, cmp=compare) -> Optional[ISeq]:
Expand All @@ -1436,35 +1440,39 @@ def sort_by(keyfn, coll, cmp=compare) -> Optional[ISeq]:
using cmp or use the `compare` fn if not.
The comparator fn can be either a boolean or 3-way comparison fn."""
if isinstance(coll, IPersistentMap):
coll = lseq.to_seq(coll)
seq = lseq.to_seq(coll)
if seq:
if isinstance(coll, IPersistentMap):
coll = seq

comparator = _fn_to_comparator(cmp)
comparator = _fn_to_comparator(cmp)

class key:
__slots__ = ("obj",)
class key:
__slots__ = ("obj",)

def __init__(self, obj):
self.obj = obj
def __init__(self, obj):
self.obj = obj

def __lt__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) < 0
def __lt__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) < 0

def __gt__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) > 0
def __gt__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) > 0

def __eq__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) == 0
def __eq__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) == 0

def __le__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) <= 0
def __le__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) <= 0

def __ge__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) >= 0
def __ge__(self, other):
return comparator(keyfn(self.obj), keyfn(other.obj)) >= 0

__hash__ = None # type: ignore
__hash__ = None # type: ignore

return lseq.sequence(sorted(coll, key=key))
return lseq.sequence(sorted(coll, key=key))
else:
return llist.EMPTY


def is_special_form(s: sym.Symbol) -> bool:
Expand Down
12 changes: 12 additions & 0 deletions tests/basilisp/test_core_fns.lpy
Original file line number Diff line number Diff line change
Expand Up @@ -992,8 +992,14 @@
'([1] [1 2] [1 3]) [[1 3] [1] [1 2]]
'([1] [0 1] [0 1]) [[0 1] [1] [0 1] ]))

(testing "sorting sequences"
(are [res v] (= res (sort v))
'() (seq [])
'(1 2 3) (seq [2 3 1])))

(testing "sorting maps"
(are [res v] (= res (sort v))
'() {}
'([:3 18] [:5 28] [:9 23]) {:9 23 :3 18 :5 28})))

(deftest sort-by-test
Expand All @@ -1018,8 +1024,14 @@
(is (= '([1 2] [2 2] [2 3]) (sort-by first [[1 2] [2 2] [2 3]])))
(is (= '([2 2] [2 3] [1 2]) (sort-by first > [[1 2] [2 2] [2 3]]) )))

(testing "sorting seqs"
(are [res v] (= res (sort-by second > v))
'() (seq [])
'([1 7] [3 4] [2 1]) (seq [[2 1] [3 4] [1 7]])))

(testing "sorting maps"
(are [res v] (= res (sort-by identity v))
'() {}
'([:3 18] [:5 28] [:9 23]) {:9 23 :3 18 :5 28})

;; taken from clojuredocs
Expand Down

0 comments on commit 44760b6

Please sign in to comment.