Opened 5 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: sage-8.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, GitHub, GitLab) Commit: 1652bef04e5bae631c97690ab64468871daf7810
Dependencies: Stopgaps:

Status badges

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 chapoton

  • Branch set to public/21668
  • Commit set to 90ec4bb8ed7daf60cc70360434b7ab1ca086a356
  • Status changed from new to needs_review

New commits:

85ef664Mode returns sorted list
90ec4bbMerge branch 'mode-fixes' in 8.0.b4

comment:2 Changed 4 years ago by chapoton

  • Milestone changed from sage-7.4 to sage-8.0

comment:3 Changed 4 years ago by chapoton

  • Authors changed from Murray Tannock to Frédéric Chapoton
  • 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:

a24030btrac 21668 make sure the mode is sorted

comment:4 Changed 4 years ago by tscrim

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 chapoton

so, is this a positive review, Travis ?

comment:6 Changed 4 years ago by tscrim

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 git

  • Commit changed from a24030bfed5149b7b1998d68ff4273dbf02e0992 to 5755b9536a33e162e621649368595fc417631567

Branch pushed to git repo; I updated commit sha1. New commits:

e2fa54aMerge branch 'u/chapoton/21668' in 8.0.b5
5755b95trac 21668 add try except

comment:8 Changed 4 years ago by chapoton

Done. But I have no idea what you mean by "appropriate changes".

comment:9 Changed 4 years ago by tscrim

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 git

  • Commit changed from 5755b9536a33e162e621649368595fc417631567 to 1652bef04e5bae631c97690ab64468871daf7810

Branch pushed to git repo; I updated commit sha1. New commits:

1652beftrac 21668 change in doc

comment:11 Changed 4 years ago by chapoton

done


New commits:

1652beftrac 21668 change in doc

comment:12 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you.

comment:13 Changed 4 years ago by vbraun

  • Branch changed from u/chapoton/21668 to 1652bef04e5bae631c97690ab64468871daf7810
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.