Pair programming matrix

1. Comments on your code

1. No docstrings! What do these functions do and what kind of values do they return?

2. Python statements don't need to be terminated with a semicolon: they finish at the end of a line.

3. Is it really necessary to do from git import * given that you only actually use git.Repo?

4. There's a built-in function called list, so it's a bad idea to use that as the name of a variable. Also, list is a vague name: list of what? I suggest something like users.

5. Similarly, build is a vague name. Build what? I suggest something descriptive like count_commit_pairings.

6. It's conventional (following PEP8) for Python variables to use lower case with underscores. If you plan to collaborate with other Python programmers, use variable names like pairs_dict and paired_with instead of pairsDict and pairedWith.

7. Constants like '2013-02-06+14:00' shouldn't be buried deep in the code, but should be pulled out to become global variables or function parameters.

8. You could avoid having to write these lines:

   if person not in pairsDict:
       pairsDict[person] = Counter()
   

   and these lines:

   if pairedWith not in pairsDict:
       pairsDict[pairedWith] = Counter()
   

   if you made pairsDict into a defaultdict(Counter). See collections.defaultdict.

9. Since you're going to be looking up users to see whether they need to be replaced with 'Other', it's good practice to convert the list of users into a set, so that looking up a name can be done in constant time.

10. It's wasteful to iterate over the users and try to match each one against the commit message. Why not do a regular expression match against the commit message just once and then figure out which users are mentioned?

11. You use the line

    if person in commit.message:
    

    to determine if person is the committer. But what if someone makes a commit without writing their name in the commit message (false negative)? What if someone's name appears in the commit message by coincidence (false positive)? It seems to me that looking at commit.committer.name would be more reliable.

12. Backslash is an escape character in Python strings, so you should generally use the r modifier in front of strings where you want to preserve the backslashes. So write:

    m = re.search(r'(\w+)/(\w+)\s', commit.message)
    

13. It's better to use the regular expression \b (word boundary) rather than \s (whitespace) to detect the end of a word. This is because \b matches if the word is followed by punctuation, or by the end of the string, whereas \s would fail in those cases.

14. If the name of the pairing is supposed to be at the start of the commit message, you should use re.match to ensure this. But if it is allowed to be anywhere in the commit message you should write:

    m = re.search(r'\b(\w+)/(\w+)\b', commit.message)
    

    to ensure that the match starts at the beginning of a word.

15. Each time you call re.search it has to compile the regular expression. Since you're going to do this for each commit, it makes sense to compile the regular expression just once by calling re.compile.

16. In the section of code

    if m.group(1) == person:
        pairedWith = m.group(2) #pair is right of slash
    else:
        pairedWith = m.group(1) #pair is left of slash
    

    you assume that if person is in the message, and not to the left of the slash, then they must be to the right of the slash. But that doesn't follow at all. Suppose Gabrielle commits a change with the message

    Gabrielle/Frank -- fixed bug 123 found by Alice
    

    it looks to me as though your code will incorrectly deduce that this is a change committed by Alice and Frank. So I think it would be worth taking more care to detect (and perhaps report?) errors in the commit messages. People make mistakes!

2. Revised build function

Addressing all the issues discussed above yields the following code. But beware: I haven't tested this, so most likely there are mistakes.

from git import Repo
import re
from collections import Counter, defaultdict
import sys

def count_commit_pairings(repository = '/home/developer/work/commerce-git/commerce-project',
                          since = '2013-02-06+14:00',
                          users = 'Alice Bill Clare David Emma Frank Gabrielle'.split()):
    """
    Count the number of times each pair of users has made a commit.

    `repository` is the git repository to examine.
    `since` is the date at which to start looking at the commits.
    `users` is a list of users to count commits for.

    Return the counts in the form of a dictionary mapping user name to
    a dictionary mapping partner name to count of commits by that
    pair. Commits not made by a pair are filed as if by the committer
    and the fake user 'Solo'. User names not found in `users` are
    replaced by 'Other'.
    """
    repo = Repo(repository)
    pairs = defaultdict(Counter)
    users = set(users)
    pair_re = re.compile(r'(\w+)/(\w+)\b')

    def username(name):
        """Return the username to use for `name`."""
        return name if name in users else 'Other'

    def partner(commit):
        """Return the partner of the committer, or 'Solo' if none found."""
        m = pair_re.match(commit.message)
        if not m:
            return 'Solo'
        if commit.committer.name not in m.groups():
            sys.stderr.write("Bad commit {0.hexsha}: committer {0.committer.name} "
                             "missing from pair {1}."
                             .format(commit, m.group(0)))
            return username(m.group(1))
        partner = set(m.groups()).difference([commit.committer.name])
        if len(partner) != 1:
            sys.stderr.write("Bad commit {0.hexsha}: duplicate pair {1}."
                             .format(commit, m.group(0)))
            return 'Solo'
        return username(partner.pop())

    for commit in repo.iter_commits(repo.heads.master, since=since, reverse=True):
        u1 = username(commit.committer.name)
        u2 = partner(commit)
        pairs[u1][u2] += 1
        pairs[u2][u1] += 1

    return pairs

(This seems like plenty for one answer. Maybe some other person can look at print_table.)