Script to access API and print formatted report to stdout

Question

I work for a non-profit that runs acoustic music camps for adults. This script queries the API for Regfox, the event registration software we use, and prints for each camp a table showing the number of spots available for each of the accommodation types we offer.

All comments appreciated, especially how to make the code more pythonic.

import requests
from collections import namedtuple

import credentials  # local file


# =========================================================================
# THIS INFO CHANGES AT START OF EACH CAMP SEASON, OTHERWISE IMMUTABLE
# ------------------------------------------------------------------------
CampInfo = namedtuple("CampInfo", ["full_name", "regfox_page_id"])
MANDO_CAMP = CampInfo("2023 Mandolin Camp North", credentials.MC_REGFOX_PAGE_ID)
BANJO_CAMP = CampInfo("2023 Banjo Camp North", credentials.BC_REGFOX_PAGE_ID)

ColumnWidth = namedtuple("ColumnWidth", ["name", "width"])
name = ColumnWidth("Name", 47)
supply = ColumnWidth("Supply", 7)  # these three are number of chars + 1
sold = ColumnWidth("Sold", 5)
available = ColumnWidth("Available", 10)

ROOM_TYPES = (
    "Single room, private bath, Hilltop campus",
    "Single room, shared bath, Pondside campus",
    "Double room, private bath, Hilltop campus",
    "Double room, shared bath, Pondside campus",
    "Quad room, shared bath, Pondside campus",
    "Double room, [NON-PARTICIPATING CAMPER], private bath, Hilltop campus",
    "Double room, [NON-PARTICIPATING CAMPER], shared bath, Pondside campus",
    "Commuter / RV",
    "Commuter / RV, [NON-PARTICIPATING CAMPER]",
    "Friday",
    "Saturday",
    "Sunday",
    "Friday evening",
    "Saturday evening",
)
# =========================================================================


def print_row(a, b, c, d):
    """Print a row in a table"""
    for text, align, width in zip(
        [a, b, c, d],
        "<>>>",
        [name.width, supply.width, sold.width, available.width],
    ):
        print(
            "{text:{align}{width}}".format(text=text, align=align, width=width),
            end="",
        )
    print()


def print_header(camp):
    """Print a header for a camp"""
    separator_length = 69
    print()
    print("=" * separator_length)
    print(camp.full_name)
    print("-" * separator_length)
    print_row("Room type", "Supply", "Sold", "Available")
    print("-" * separator_length)


def get_inventory(regfox_page_id):
    """Get data for each kind of room via Regfox API"""
    url_string = (
        "https://api.webconnex.com/v2/public/forms/" + regfox_page_id + "/inventory"
    )
    try:
        r = requests.get(
            url=url_string,
            headers={
                "apiKey": credentials.API_KEY,
            },
        )
        return r.json()
    except requests.exceptions.RequestException:
        print("HTTP Request failed")


def create_table(camp):
    """Create a table for one camp"""
    inventory_raw = get_inventory(camp.regfox_page_id)
    inventory = inventory_raw["data"]
    for room_type in ROOM_TYPES:
        for i in inventory:
            if i["name"] == room_type:
                available = i["quantity"] - i["sold"]

                # shorten long names
                if "NON-PARTICIPATING" in i["name"]:
                    i["name"] = i["name"].replace(
                        "[NON-PARTICIPATING CAMPER]", "NON-PART"
                    )
                i["name"] = i["name"].replace(" campus", "")

                print_row(i["name"], i["quantity"], i["sold"], available)


def main():
    for camp in [MANDO_CAMP, BANJO_CAMP]:
        print_header(camp)
        create_table(camp)
        print()


if __name__ == main():
    main()

Answer 1

In many ways this is some very nice code.

The author has clearly exercised great care when trying to make it readable & maintainable by others.

There are some structural aspects that could be improved, the biggest of which is dealing generically with tables.

---

CampInfo = namedtuple("CampInfo", ["full_name", "regfox_page_id"])
MANDO_CAMP = CampInfo("2023 Mandolin Camp North", credentials.MC_REGFOX_PAGE_ID)
BANJO_CAMP = CampInfo("2023 Banjo Camp North", credentials.BC_REGFOX_PAGE_ID)

I really like this approach to solving "same problem" for two camps.

---

supply = ColumnWidth("Supply", 7)  # these three are number of chars + 1
sold = ColumnWidth("Sold", 5)
available = ColumnWidth("Available", 10)

The magic number 47 is apparently a tuning parameter related to your data and your paper width or video display width; I'm willing to call it inevitable for now. It has a nice clear name.width label, good.

For the code as written, the comment is very helpful, thank you. But DRY. I am sad that we're specifying 7, 5, 10, rather than obtaining them as 1 + len() of string constant results.

Pep-8 asks that you capitalize these class names: Name, Supply, Sold, Available.

    separator_length = 69

Kudos, this is the perfect way to deal with a magic number. (Assuming it must be hardcoded, rather than dynamically computed. Or even found as a sum of other static magic numbers.)

---

def get_inventory(regfox_page_id):
    """Get data for each kind of room via Regfox API"""
    ...
        return r.json()
    except requests.exceptions.RequestException:
        print("HTTP Request failed")

Yay, we have a docstring!

There is a single business concept, a room type, which here we refer to as a room kind. On initial reading I utterly failed to understand what the docstring was trying convey to me. From the singular "each" I got the impression that page ID encoded room type and we would return records for just one of the 14 room types. Recommend you rephrase it as "Get data for all room types...", since it turns out that page ID encodes the camp rather than the room type of interest.

Getting the URL looks beautiful. But it might have errored. And then we swallow the error and soldier on, having reported it to stdout.

For the benefit of future maintainers, it might be nice to mention which URL failed.

Consider discarding the try and do this right after the GET:

        r.raise_for_status()
        return r.json()

That is, consider making errors fatal. It's not clear to me that the business can usefully consume a "partial" report.

Consider doing return r.json()["data"], since that's all the consumer cares about anyway.

---

    for room_type in ROOM_TYPES:
        for i in inventory:
            if i["name"] == room_type:

Making 14 passes over the API result is not end of the world, but it is slightly unfortunate. There's a couple of things going on here, and we would like for create_table to stick to a single responsibility. So consider

1. writing a helper that generates rows in the desired order, and
2. using sorted() to produce that order.

Here is what I have in mind:

    for row in _get_sorted_rows(inventory):
        available = row["quantity"] - row["sold"]
        ...

from functools import lru_cache

def _get_sorted_rows(inventory):
    """Puts same room type together.

    Due to stable sort, row order is otherwise preserved."""
    return sorted(inventory,
                  key=lambda row: _room_type_index(row["name"]))

@lru_cache
def _room_type_index(room_type: str) -> int:
    return ROOM_TYPES.index(room_type)

---

                # shorten long names

Thank you for the helpful comment.

The fact that you found it necessary to write it suggests that we should revisit the Single Responsibility topic and break out a def _shorten(long_name): helper.

---

def print_row(a, b, c, d): ...
    for text, align, width in zip(
        [a, b, c, d],
        "<>>>",
        [name.width, supply.width, sold.width, available.width],

The generic parameter names is kind of nice. It suggests this is a pretty general purpose routine. But then it turns out there's this peculiar relationship to the static magic width figures. Maybe caller should pass in four arguments which each have a width attached? Maybe pass in a vector of widths?

Kudos on the zip() and the <>>>, that's just lovely.

---

dynamic widths

Maybe there are aesthetic or business reasons why the column widths must be what they are. But maybe we could compute them dynamically from the data?

Your table is a list of rows, a list of dicts. Processing phases would be:

1. table = _get_rows(...)
2. widths = _find_max_column_widths(table)
3. display_table(table, widths)

---

This codebase achieves its design goals.

I would be willing to delegate or accept maintenance tasks on it.