12
\$\begingroup\$

I assigned the following contrived problem in a comparative programming languages course to give students practice with "streaming":

Write function that returns the top ten players by points-per-game among the players that have been in 15 games or more. The input to your function will be an object, keyed by team, with a list of player stats. Each player stat is an array with the player name, the number of games played, and the total number of points.

A sample data set is as follows:

stats = {
    'ATL': [
        ['Betnijah Laney', 16, 263],
        ['Courtney Williams', 14, 193],
    ],
    'CHI': [
        ['Kahleah Copper', 17, 267],
        ['Allie Quigley', 17, 260],
        ['Courtney Vandersloot', 17, 225],
    ],
    'CONN': [
        ['DeWanna Bonner', 16, 285],
        ['Alyssa Thomas', 16, 241],
    ],
    'DAL': [
        ['Arike Ogunbowale', 16, 352],
        ['Satou Sabally', 12, 153],
    ],
    'IND': [
        ['Kelsey Mitchell', 16, 280],
        ['Tiffany Mitchell', 13, 172],
        ['Candice Dupree', 16, 202],
    ],
    'LA': [
        ['Nneka Ogwumike', 14, 172],
        ['Chelsea Gray', 16, 224],
        ['Candace Parker', 16, 211],
    ],
    'LV': [
        ['A’ja Wilson', 15, 304],
        ['Dearica Hamby', 15, 188],
        ['Angel McCoughtry', 15, 220],
    ],
    'MIN': [
        ['Napheesa Collier', 16, 262],
        ['Crystal Dangerfield', 16, 254],
    ],
    'NY': [
        ['Layshia Clarendon', 15, 188]
    ],
    'PHX': [
        ['Diana Taurasi', 13, 236],
        ['Brittney Griner', 12, 212],
        ['Skylar Diggins-Smith', 16, 261],
        ['Bria Hartley', 13, 190],
    ],
    'SEA': [
        ['Breanna Stewart', 16, 317],
        ['Jewell Loyd', 16, 223],
    ],
    'WSH': [
        ['Emma Meesseman', 13, 158],
        ['Ariel Atkins', 15, 212],
        ['Myisha Hines-Allen', 15, 236],
    ],
}

Now in JavaScript, there is a "fluent" or method-chaining style readily apparent:

function topTenScorers(stats) {
  return Object.entries(stats)
    .flatMap(([team, players]) => players.map(player => [...player, team]))
    .filter(([, games, ,]) => games >= 15)
    .map(([name, games, points, team]) => ({ name, ppg: points / games, team }))
    .sort((p1, p2) => p2.ppg - p1.ppg)
    .slice(0, 10)
}

However, my Python solution (below) just doesn't satisfy the same way (I'm more of a JavaScript programmer). I've heard that Python list comprehensions are preferred to map and filter; I think that Python doesn't have a built-in flat_map, and, well, although you can do fancy things with itertools, Pythonic programs tend to, I think, be more favorable to computing intermediate expressions than to chaining. So I came up with the following:

def top_ten_scorers(stats):
    with_teams = [[*player, team]
                for (team, players) in stats.items()
                for player in players]
    with_ppg = [{'name': name, 'ppg': points/games, 'team': team}
                for [name, games, points, team] in with_teams
                if games >= 15]
    return sorted(with_ppg, key=lambda k: k['ppg'], reverse=True)[:10]

I'd love to know whether the code is in the style of current Python best practices. I know Python is well-loved by data scientists, and this problem, although very contrived, feels data-sciencey to me, so I figured a set of best practices would have arisen that my code might not meet. Also, I'm having trouble with names for the intermediate expressions, and am not sure whether the breakdown of steps is too coarse or too fine. I'm not sure which approach to take to clean it up.

Of course, it is not imperative that a streaming solution be found; what is most important is a solution that best fits the Zen of Python rule(s) "There should be one-- and preferably only one --obvious way to do it. Although that way may not be obvious at first unless you're Dutch."

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Note that something that makes all solutions inevitably a bit more clunky than necessary is that your input encodes player properties (name, number of games, points) by position and not by key or similar. Without it, one could write a readable solution that does not require wrapping the processed player stats featuring points per game in a dictionary. \$\endgroup\$
    – Wrzlprmft
    Sep 22, 2020 at 8:32
  • \$\begingroup\$ Yes, very true! That was intentional :) Not trying to purposely frustrate students, but to give them a hint of "messiness" in the data \$\endgroup\$
    – Ray Toal
    Sep 23, 2020 at 0:51

3 Answers 3

17
\$\begingroup\$

This is more "streaming" in a sense:

from heapq import nlargest
from operator import itemgetter

def top_ten_scorers(stats):
    players = (dict(name=name, ppg=points/games, team=team)
               for team, players in stats.items()
               for name, games, points in players
               if games >= 15)
    return nlargest(10, players, key=itemgetter('ppg'))

Your with_teams and with_ppg are fully computed lists, and then sorted creates another one that it then sorts, and then you throw away all but ten elements of it.

My players is a generator iterator, computing more elements on the fly as requested. The players = ... assignment only sets up the iterator, but nothing gets processed yet.

Then nlargest consumes players one by one, keeping only the top 10 seen so far and returning them sorted (in descending order). Could also be more efficient than sorting everything, depending on the number of eligible players.

I actually found your first two steps more confusing than helpful, as your with_teams creates an intermediate result/format to understand. I think it's simpler and easier to read to just directly produce the player dicts from the stats. Then again, I might be biased towards this and away from yours because I'm used to Python, which, as you say, isn't much into chaining.

Btw, here's an old message from Guido about some forms of chaining. Not sure it relates to what we have here, but perhaps interesting anyway.

I used dict(...) just for brevity, but {...} is faster, so you might want to keep the latter:

Setup:
name, ppg, team = 'Betnijah Laney', 263/16, 'ATL'

Round 1:
  347.041 ns  dict(name=name, ppg=ppg, team=team)
  128.325 ns  {'name': name, 'ppg': ppg, 'team': team}

Round 2:
  350.576 ns  dict(name=name, ppg=ppg, team=team)
  129.106 ns  {'name': name, 'ppg': ppg, 'team': team}

Round 3:
  347.753 ns  dict(name=name, ppg=ppg, team=team)
  130.734 ns  {'name': name, 'ppg': ppg, 'team': team}
\$\endgroup\$
6
  • \$\begingroup\$ Excellent point about my use of lists for the intermediate results instead of generator expressions (facepalm, I did know about them), and the Guido remark was helpful (the mutable sort in the JS chain bothered me a tiny bit I must confess). \$\endgroup\$
    – Ray Toal
    Sep 21, 2020 at 20:49
  • \$\begingroup\$ There are usually better data structures than lists of dicts. If you have to iterate over the whole list if you want to find anything. For the above example, a dict with names as keys and a tuple of ppg and team might be better suited. Something like {name: (points / games, team) for team, rows in stats.items() for name, games, points in rows}, for example. \$\endgroup\$ Sep 22, 2020 at 14:29
  • \$\begingroup\$ @EricDuminil But should I then store Michelle Campbell or should I store Michelle Campbell? \$\endgroup\$ Sep 22, 2020 at 15:41
  • \$\begingroup\$ @superbrain you mean, in case of name conflicts? Yes, that's one potential problem. \$\endgroup\$ Sep 22, 2020 at 15:44
  • 1
    \$\begingroup\$ @EricDuminil Yes, the example are apparently WNBA players and Michelle Campbell is the first duplicate name here. They do link to the same page, though, so either it's just an erroneous duplicate entry and there really is only one player with that name, or that system assuming that duplicate names can't happen assumed wrong and perhaps did something like you suggested and shouldn't have :-P \$\endgroup\$ Sep 22, 2020 at 15:48
6
\$\begingroup\$

It is possible to write those steps in a single comprehension -- sort of the Python analogue for chaining in JavaScript or Ruby. It doesn't read too badly if you convey the logic visually. Without that attention to code layout, too much burden would be placed on the readers and maintainers.

from operator import itemgetter

def top_ten_scorers(stats):
    return sorted(
        (
            dict(
                name = name,
                team = team,
                ppg = points / games,
            )
            for team, players in stats.items()
            for name, games, points in players
            if games >= 15
        ),
        reverse = True,
        key = itemgetter('ppg'),
    )[:10]

I would probably break it down more explicitly into the 3 steps: organize data; order it; select top 10.

def top_ten_scorers2(stats):
    players = [
        dict(
            name = name,
            team = team,
            ppg = points / games,
        )
        for team, players in stats.items()
        for name, games, points in players
        if games >= 15
    ]
    ranked = sorted(players, reverse = True, key = itemgetter('ppg'))
    return ranked[:10]
\$\endgroup\$
9
  • 2
    \$\begingroup\$ Using dict like that isn't normal. \$\endgroup\$
    – Peilonrayz
    Sep 21, 2020 at 10:13
  • 3
    \$\begingroup\$ @Peilonrayz It may not be common, but I think it's pretty clear, which matters most imo \$\endgroup\$
    – Energya
    Sep 21, 2020 at 11:28
  • 3
    \$\begingroup\$ @Energya So would just sticking with a literal. \$\endgroup\$
    – Peilonrayz
    Sep 21, 2020 at 11:42
  • 4
    \$\begingroup\$ @Peilonrayz especially when names are fixed, using the dict() call means getting rid of all the single quotes, which I think improves the readability here. Nothing would be wrong with the {'name': name, ...}-style literal either btw, matter of preference \$\endgroup\$
    – Energya
    Sep 21, 2020 at 11:53
  • 1
    \$\begingroup\$ @EricDuminil I'm not sure we're on the same page. List of dicts is like the staple for REST frameworks, no problem here. My comment is about using dict with kwargs rather than {}. \$\endgroup\$
    – Peilonrayz
    Sep 22, 2020 at 14:49
1
\$\begingroup\$

I will state from the beginning that I do not necessarily think that such a 'semi-functional' style is "better" than the nested list comprehensions in the accepted answer, which also have a certain nice 'fluid' / 'chain' vibe to them (as per OP's words).

However, I'm adding this answer to point out that if the kind of semi-functional / 'chaining' style OP demonstrated via Javascript is preferred, then this is entirely possible in python as well (though it might require defining a couple of extra helper functions to enable it).

Here is an example below. First, since python does not have a bespoke 'chain' (a.k.a 'pipe') operator, we create a very simple one (taken from here):

def chain( Accumulant, *Functions_list ):
    for f in Functions_list: Accumulant = f( Accumulant )
    return Accumulant

Let's also create a simple, curried, reduce function, so that we can perform map -> reduce instead of flatmap:

def reduce_f( Function ):
    def reductor (List):
        while len( List ) > 1: List.insert( 0, Function( List.pop(0), List.pop(0) ) )
        return List[0]
    return reductor

Finally, let's create functional, curried versions of a couple of standard functions which we want to use. Note that this is not necessary, and the lambdas defined here could have been dumped directly in the 'chain', but predefining them here makes things much easier to the eye, and I have chosen these names/functions so that they are directly comparable to the javascript code functionality in the question:

splat_f  = lambda f: lambda t: f(*t)   # explode a tuple and pass it as arguments to f
map_f    = lambda f: lambda _: list( map( f, _ ) )
filter_f = lambda f: lambda _: list( filter( f, _ ) )
sorted_f = lambda f: lambda _: sorted(_, key=f )
slice_f  = lambda start, stop, step=1: lambda l: l[slice(start, stop, step)]

Armed with the above, we can re-create the equivalent "fluent", method-chaining style in python. It looks almost identical:

def topTenScores( stats ):
  return chain( stats
    , dict.items, list
    , map_f( splat_f(lambda team, players: list(map(lambda player: [*player, team], players))))
    , reduce_f( list.__add__ )
    , filter_f( splat_f(lambda _1, games, _2, _3:  games >= 15) )
    , map_f( splat_f(lambda name, games, points, team:{'name':name,'ppg':points/games,'team':team}))
    , sorted_f( lambda x : x['ppg'] )
    , slice_f( 0, 10 )
  )
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.