Cleaning a game logs list to find the frequent action triplets and the busy user

Question

I have these game logs which I need to clean, process and find the frequency of game actions. These game actions should be a triplet. In the below given list, "1|123|/jump", "2|123|/flip", "3|123|/crouch" is a triplet and also "2|123|/flip", "3|123|/crouch", "8|123|/dance".
Similarly, "4|999|/jump", "5|999|/flip", "6|999|/crouch", "5|999|/flip", "6|999|/crouch", "7|999|/stroll", and "6|999|/crouch", "7|999|/stroll", "9|999|/jump".
And finally, "10|639|/flip", "11|639|/crouch","12|639|/stroll".

The output should be a dictionary of action triplets with keys as action triplets and value is the frequency of the action triplets.

{'/jump,/flip,/crouch': 2, '/flip,/crouch,/dance': 1, '/flip,/crouch,/stroll': 2, '/crouch,/stroll,/jump': 1}

and the user_id which has the highest number of action triplets. In this case it is user_id: 999

The following is my approach:

1. I split the string on | and create a list of tuples (user_id, action). I ignore the sequence id (first part of the log).
2. I sort the list of tuples by user_id.
3. I create triplets by comparing the three consecutive user_ids and create a 2-d list.
4. Create a dictionary with key (append each user_action together with a comma) from each list of action triplets and value being the count of these same set of user_actions.
5. Create another dictionary with key (user_id) - I took the very first value of the tuple from each list as each triplet list can only have one user_id.
6. Then, I return the max value key i.e., user_id at the end.
   

    game_logs = ["1|123|/jump", "2|123|/flip", "3|123|/crouch", "4|999|/jump", "5|999|/flip", "6|999|/crouch", "7|999|/stroll", "8|123|/dance", "9|999|/jump", "10|639|/flip", "11|639|/crouch","12|639|/stroll"]
    
    def clean(game_logs):
        size = len(game_logs)
        user_id_actions = []
    
        for game_log in game_logs:
            id, user_id, action = extract(game_log)
            user_id_actions.append((user_id, action))
    
        user_id_actions.sort(key = lambda x:x[0])
        # print(user_id_actions)
        
        action_triplets = []
        for i in range(size - 2):
            if user_id_actions[i][0] == user_id_actions[i+1][0] and user_id_actions[i+1][0] == user_id_actions[i+2][0]:
                action_triplets.append([user_id_actions[i], user_id_actions[i+1], user_id_actions[i+2]])
        
        # print(action_triplets)
        
        triplet_hash = {}
        max_actions_user_id_hash = {}
        for triplet in action_triplets:
            page_string = []
            for val in triplet:
                user_id, page = val
                page_string.append(page)
            fin_string = ','.join(page_string)
            if fin_string not in triplet_hash:
                triplet_hash[fin_string] = 1
            else:
                triplet_hash[fin_string] += 1
        
        # print(triplet_hash)
        print(f"Action action_triplets: {triplet_hash}")  
        
        for triplet in action_triplets:
            user_id = triplet[0][0]
            if user_id not in max_actions_user_id_hash:
                max_actions_user_id_hash[user_id] = 1
            else:
                max_actions_user_id_hash[user_id] += 1
        
        # print(max_actions_user_id_hash)
        
        print(f"Highest number of action_triplets are for the user_id :{max(max_actions_user_id_hash, key=max_actions_user_id_hash.get)}")   
            
        
    
    def extract(game_log):
        extract_info = tuple(game_log.split('|'))
        return extract_info
    
    clean(game_logs)

It feels like I over complicated the whole logic here. Creating nested lists, creating multiple dictionaries. Could anyone share feedback.

Thank you

Answer 1

Your code is reasonable. The primary advice I have focuses not on anything particularly wrong with your current implementation but on simpler ways to complete the task using the standard library.

The first part of your function assembles the data into triples, and you're on the right track in the sense that you are sorting the data by user ID. But you can simplify further by taking advantage of itertools.groupby. Here's a function that takes the game logs and returns a dict mapping user ID to all of that user's action tuples.

from operator import itemgetter
from itertools import groupby
from collections import Counter

def parse_game_logs(game_logs):
    # Parse logs into tuples. No need to abandon the sequence ID.
    actions = [glog.split('|') for glog in game_logs]

    # Prepare the grouping generator.
    uidkey = itemgetter(1)
    grouping_gen = groupby(sorted(actions, key = uidkey), key = uidkey)

    # Return dict mapping user ID to its actions.
    return {
        uid : list(acts)
        for uid, acts in grouping_gen
    }

Once you have that dict, finding the user with the most actions involves finding the minimum (key, value) tuple, where minimum computation is based on length of the value.

def get_most_active_user(actions):
    len_key = lambda tup: len(tup[1])
    return max(actions.items(), key = len_key)[0]

And the same dict of actions can be used to assemble the tally of action triples. A Counter is a good data structure to use:

def get_triple_frequencies(actions):
    return Counter(
        ','.join(a for sid, uid, a in acts[i : i + 3])
        for uid, acts in actions.items()
        for i in range(len(acts) - 2)
    )

Putting all of the pieces together:

def clean(game_logs):
    actions = parse_game_logs(game_logs)
    return (
        get_most_active_user(actions),
        get_triple_frequencies(actions),
    )

Answer 2

FMc put it well in his commendable job:
Your code is reasonable.

My main gripe:

The code does not document what it is to accomplish.
The Style Guide for Python Code cautions Comments that contradict the code are worse than no comments. - to which I'd add where the code is right. (One might with no less justification state code that contradicts correct comments is worse than no code.)
But how does one know the code is right?
• correct code passes tests
• correct code fully implements a specification (parts specified optional exempted)
Python has got it right in specifying a documentation convention that makes it trivial to copy that part of documentation with the code (you'd have to delete it) and allows introspection.

---

Odds&ends:

• the name conventionally used to receive values not intended for use is _:
  _, user_id, action = extract(game_log)
• when looking for runs of k elements in a sorted/grouped iterable, it suffices to check first and last item for equivalence:
  if user_id_actions[i][0] == user_id_actions[i + k-1][0]:
  • Python has a shorthand for the conjunction of multiple comparisons involving an expression, say, actions[1][0], twice:
    inclusive <= actions[1][0] < exclusive (fits bounds checks)
• it's easy to not include the (unwanted) id in the sequences to group:
  actions = tuple(log_entry.split('|', 3)[1:] for log_entry in game_logs)
• I'm with print("action triplets", triplet_hash)

An exercise in not keeping what is not needed - be sure to compare readability:

from operator import itemgetter
from collections import Counter

def action_triplet2count_and_busiest_user(entries):
    """ Return a Counter of action triplets (same user,
                            same three consecutive actions)
               and the ID of the user accounting 
                       for the highest number of action triplets
    """
    triplet2count = Counter()
    busyboy, busy = None, 0
    # Parse log entries into sequences. No need to keep the entry ID.
    sorted_actions = [(parts[1], parts[2]) for parts in (
                      entry.split('|', 3) for entry in entries)]
    sorted_actions.sort(key=itemgetter(0))  # specified stable
    id, actions = None, tuple()
    sorted_actions.append((id, id))  # ease handling end of sequence
    for user, action in sorted_actions:
        if id != user:
            triplets = len(actions) - 2
            if 0 < triplets:
                actions = tuple(actions)
                # resources [i:i+3] vs. str.join()?
                triplet2count.update(actions[i:i+3]
                                     for i in range(triplets))
                if busy < triplets:
                    busyboy, busy = id, triplets
            id, actions = user, []
        actions.append(action)
    
    return triplet2count, busyboy