GitHub repo tree generator

Question

I have built a generator that, using the GitHub API, creates a dictionary containing a tree of all the resources in any GitHub repo. It uses the function git_tree, which takes one argument, a repo-string (repo-author/repo-name). For example, this repo only contains one file, a README. If we pass in the repo-string, and format the end result, this is what we get:

>> json.dumps(git_tree("githubtraining/github-move"), indent=3)
{
   "github-move": {
      "files": [
         "README.md"
      ],
      "dirs": {}
   }
}

You can see that it returns a dict with a key/value pair of {repo_name: tree}. So the github_move item contains a list of all files in that directory, and a dict with more nested directories. Obviously, in this repository there aren't any other directories, so that dict is just blank.

For sake of purpose, here is the tree of this repo (it was too long to put in the post). You can see each directory and subdirectory has its own files list and dirs dict.

Here's the code (repl.it online program for testing):

import requests
from pprint import pprint
from functools import reduce
import operator
import json
from itertools import chain, repeat, islice

class GitError(Exception): pass

def intersperse(delimiter, seq):
  return list(islice(chain.from_iterable(zip(repeat(delimiter), seq)), 1, None))

def _get_from_dict(dataDict, mapList):
  return reduce(operator.getitem, mapList, dataDict)

def _append_in_dict(dataDict, mapList, value):
  _get_from_dict(dataDict, mapList[:-1]).append(value)

def _get_sha(author, repo):
  try:
    return requests.get('https://api.github.com/repos/{}/{}/branches/master'.format(author, repo)).json()['commit']['commit']['tree']['sha']
  except KeyError as ex:
    raise GitError("Invalid author or repo name") from ex

def _get_git_tree(author, repo):
  return requests.get("https://api.github.com/repos/{}/{}/git/trees/{}?recursive=1".format(author, repo, _get_sha(author, repo))).json()["tree"]

def git_tree(repostring):
  author, repo = repostring.split("/")
  tree = {repo: {"files": [], "dirs": {}}}
  for token in _get_git_tree(author, repo):
    if token["type"] == "tree" and "/" not in token["path"]:
      tree[repo]["dirs"].update({token["path"]: {}})
      tree[repo]["dirs"][token["path"]].update({"files": [], "dirs": {}})
    elif token["type"] == "tree" and "/" in token["path"]:
      temp_dict = {}
      a = list(reversed(token["path"].split("/")))
      for k in a[:-1]:
        temp_dict = {k: {"files": [], "dirs": temp_dict}}
      tree[repo]["dirs"][a[-1]]["dirs"] = temp_dict
    elif token["type"] == "blob":
      path = token["path"].split("/")
      if len(path) == 1:
        tree[repo]["files"].append(path[0])
      else:
        dict_path = [repo, "dirs"] + intersperse("dirs", path[:-1]) + ["files", path[-1]]
        _append_in_dict(tree, dict_path, dict_path[-1])
  return tree

print(json.dumps(git_tree("githubtraining/caption-this"), indent=3))

(The json.dumps is just there for easy viewing, it can be ommited).

My questions:

1. Is it too messy? I look back at this function and it looks a bit cluttered and all over the place. Is that the case, or am I just going crazy?

2. Do I have any unnecessary code in there?

3. Is there anything else you deem wrong with the program?

Answer 1

@Reinderien's answer makes a very good point about docstrings and using those to describe your code: it would be much easier to understand if I knew what the various components are generally intended to do. There's a PEP about docstring formatting: PEP 257. The comments in the REPL.it link are pretty good except normal comments should come the line before the thing they're commenting, not at the right side of the line.

You could use a more descriptive name than temp_dict: at first I was very confused because you iterated over a dict, continually reassigning it for no reason, but then I realized you were recursively building a tree. I would suggest child_dict.

Similarly, a is not a very descriptive variable name. Even though it's only used once, you should still name it something slightly more descriptive, even if it's only 1 word, like path.

This is a relatively minor DRY infraction, but you use the conditional structure:

if token["type"] == "tree" and "/" not in token["path"]:
  # path 1 ...
elif token["type"] == "tree" and "/" in token["path"]:
  # path 2 ...

when you should use the structure:

if token["type"] == "tree":
  if "/" not in token["path"]:
    # path 1
  else:
    # path 2

The second form is preferred both because it's (trivially) more efficient, but more importantly, it makes the dichotomy between the two paths clear for future modification and maintenance. If you pay close attention to your control flow in general, you will often find simplifications like this.

General miscellaneous improvements

You imported pprint but never used it. You should remove unused imports. Code linters like PyLint help you avoid things like this.

You should add an if __name__ == '__main__' guard to the bottom of your program, so that people can import your program without running the example. This is a general good practice you should almost always follow.

2 spaces is not my preferred indentation, and PEP 8, the widely-used style guide for Python, happens to agree: "Use 4 spaces per indentation level". It doesn't really make that much difference, but it is slightly easier to read if you're used to it, and it's the style I see other Python programmers use. I do recommend checking out PEP 8 and starting to learn its suggestions if you haven't already, because it helps create more standard looking code.

Answer 2

I see nothing wrong with the code itself, but you're in dire need of docstrings. Arcane functional one-liners like the one in intersperse are impenetrable unless they're well-documented. You'd also benefit from splitting that one into multiple lines.

For strings like this:

    return requests.get('https://api.github.com/repos/{}/{}/branches/master'.format(author, repo)).json()['commit']['commit']['tree']['sha']

consider rewriting your format call as an f-string; i.e.

f'https://api.github.com/repos/{author}/{repo}/branches/master'