Opened 4 years ago
Closed 4 years ago
#21668 closed defect (fixed)
sage.stats.basic_stats.mode doesn't sort
Reported by:  schilly  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  statistics  Keywords:  
Cc:  tscrim, jmantysalo  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  1652bef (Commits)  Commit:  1652bef04e5bae631c97690ab64468871daf7810 
Dependencies:  Stopgaps: 
Description
The sage.stats.basic_stats.mode
function claims to sort the result, but it doesn't.
sage: my_list = [0, 2, 7, 7, 13, 20, 2, 13] sage: mode(my_list) [2, 13, 7]
Here is a possible fix by Murray Tannock
def mode(v): """ Return the mode of `v`. The mode is the sorted list of the most frequently occurring elements in `v`. If `n` is the most times that any element occurs in `v`, then the mode is the sorted list of elements of `v` that occur `n` times. NOTE: The elements of `v` must be hashable and comparable. INPUT:  `v`  a list OUTPUT:  a list EXAMPLES:: sage: v = [1,2,4,1,6,2,6,7,1] sage: mode(v) [1] sage: v.count(1) 3 sage: mode([]) [] sage: mode([1,2,3,4,5]) [1, 2, 3, 4, 5] sage: mode([0, 2, 7, 7, 13, 20, 2, 13]) [2, 7, 13] sage: mode(['sage', 4, I, 3/5, 'sage', pi]) ['sage'] sage: class MyClass: ... def mode(self): ... return [1] sage: stats.mode(MyClass()) [1] """ if hasattr(v,'mode'): return v.mode() from operator import itemgetter freq = {} for i in v: if i in freq: freq[i] += 1 else: freq[i] = 1 s = sorted(freq.items(), key=itemgetter(1,0)) return [i[0] for i in s if i[1] == s[1][1]]
Change History (13)
comment:1 Changed 4 years ago by
 Branch set to public/21668
 Commit set to 90ec4bb8ed7daf60cc70360434b7ab1ca086a356
 Status changed from new to needs_review
comment:2 Changed 4 years ago by
 Milestone changed from sage7.4 to sage8.0
comment:3 Changed 4 years ago by
 Branch changed from public/21668 to u/chapoton/21668
 Cc tscrim jmantysalo added
 Commit changed from 90ec4bb8ed7daf60cc70360434b7ab1ca086a356 to a24030bfed5149b7b1998d68ff4273dbf02e0992
I made my own branch. This should be an easy review.
New commits:
a24030b  trac 21668 make sure the mode is sorted

comment:4 Changed 4 years ago by
I don't think they necessarily have to be comparable, only to have sorted output. However, this will require catching an exception to return the unsorted list, but that is okay IMO.
comment:5 Changed 4 years ago by
so, is this a positive review, Travis ?
comment:6 Changed 4 years ago by
I would instead say you should do
try: return sorted(u for u, f in freq.items() if f == n) except TypeError: return [u for u, f in freq.items() if f == n]
with appropriate changes to the docstring.
comment:7 Changed 4 years ago by
 Commit changed from a24030bfed5149b7b1998d68ff4273dbf02e0992 to 5755b9536a33e162e621649368595fc417631567
comment:8 Changed 4 years ago by
Done. But I have no idea what you mean by "appropriate changes".
comment:9 Changed 4 years ago by
We no longer require the elements are comparable and we can say we sort this list if possible.
comment:10 Changed 4 years ago by
 Commit changed from 5755b9536a33e162e621649368595fc417631567 to 1652bef04e5bae631c97690ab64468871daf7810
Branch pushed to git repo; I updated commit sha1. New commits:
1652bef  trac 21668 change in doc

comment:11 Changed 4 years ago by
comment:12 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Thank you.
comment:13 Changed 4 years ago by
 Branch changed from u/chapoton/21668 to 1652bef04e5bae631c97690ab64468871daf7810
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Mode returns sorted list
Merge branch 'modefixes' in 8.0.b4