1
\$\begingroup\$

As part of a university assignment, I have been tasked with populating and rendering a provided Django template with COVID-19-related quarantine data retrieved from a government API. To challenge myself, I wanted to write the code in a professional manner (to the best of my abilities): e.g., using features such as type hints, yield expressions, conditional expressions, chained assignments.

Now, I am looking to evaluate my success in that regard as well as aspects I can improve going forward.

The essence of the task is getting the relevant data from the API and manipulating it into the format expected by the template. An imposed constraint is that only data from the previous seven days are acceptable.

preview

The *.py files have been formatted using Black and isort.

  • get_data.py: Module authored by me and seeking code review for.
import json
from datetime import date, timedelta
from typing import Iterator, List, Literal, Optional, Tuple

import requests


def _generate_valid_dates() -> Iterator[str]:
    """[yesterday, yesterday - 7)"""

    YESTERDAY = date.today() - timedelta(days=1)
    for i in range(7):
        next_date = YESTERDAY - timedelta(days=i)
        yield next_date.strftime("%d/%m/%Y")


def _call_api(
    dataset: Literal["occupancy", "confines"], date: str
) -> Optional[List[dict]]:
    """
    - Response JSON body parsed as a list of dict(s) or `None` if connection error.
    - `date` expects `f"%d/%m/%Y"`.
    """

    ENDPOINT = "https://api.data.gov.hk/v2/filter?q="

    if dataset == "occupancy":
        QUERY = {
            "resource": "http://www.chp.gov.hk/files/misc/occupancy_of_quarantine_centres_eng.csv",
            "section": 1,
            "format": "json",
            "sorts": [[8, "desc"]],  # by units available
            "filters": [[1, "eq", [date]]],
        }

    elif dataset == "confines":
        QUERY = {
            "resource": "http://www.chp.gov.hk/files/misc/no_of_confines_by_types_in_quarantine_centres_eng.csv",
            "section": 1,
            "format": "json",
            "filters": [[1, "eq", [date]]],
        }

    url = ENDPOINT + json.dumps(QUERY)
    res = requests.get(url)

    return res.json() if res.ok else None


def _sum_all_centres(occupancy_data: List[dict]) -> Tuple[int, int, int]:
    """`(units_in_use, units_available, persons_quarantined)`"""

    units_in_use = units_available = persons_quarantined = 0

    for centre in occupancy_data:
        units_in_use += centre["Current unit in use"]
        units_available += centre["Ready to be used (unit)"]
        persons_quarantined += centre["Current person in use"]

    return units_in_use, units_available, persons_quarantined


def generate_context() -> dict:
    """```
    schema = {
        "connected": bool,
        "has_data": bool,
        "data": {
            "date": f"%d/%m/%Y",
            "units_in_use": int,
            "units_available": int,
            "persons_quarantined": int,
            "non_close_contacts": int,
            "count_consistent": bool,
        },
        "centres": [
            {"name": str, "units": int},
            {"name": str, "units": int},
            {"name": str, "units": int},
        ],
    }
    ```"""

    ctx = {}

    has_data = False
    dates = _generate_valid_dates()

    while not has_data:
        try:
            date_ = next(dates)
        except StopIteration:
            break  # all valid dates checked

        occupancy_data = _call_api("occupancy", date_)
        confines_data = _call_api("confines", date_)

        if occupancy_data is None or confines_data is None:
            ctx["connected"] = False
            break  # connection error

        ctx["connected"] = True

        try:
            (units_in_use, units_available, persons_quarantined) = _sum_all_centres(
                occupancy_data
            )

            confines_data = confines_data[0]  # singleton for one date

            count_consistent = (
                confines_data["Current number of close contacts of confirmed cases"]
                + confines_data["Current number of non-close contacts"]
                == persons_quarantined
            )

            centres = [
                {
                    "name": centre["Quarantine centres"],
                    "units": centre["Ready to be used (unit)"],
                }
                for centre in occupancy_data[:3]
            ]
        except (IndexError, KeyError):
            # IndexError: confines_data is empty
            # KeyError: some missing field in a dataset
            continue  # proceed to next date

        has_data = ctx["has_data"] = True

        ctx["data"] = {
            "date": date_,
            "units_in_use": units_in_use,
            "units_available": units_available,
            "persons_quarantined": persons_quarantined,
            "non_close_contacts": confines_data["Current number of non-close contacts"],
            "count_consistent": count_consistent,
        }
        ctx["centres"] = centres
    return ctx
  • dashboard3.html: Template provided with the assignment.
<!doctype html>
<html>
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>Quarantine Centres Dashboard</title>
    <link href="https://fonts.googleapis.com/icon?family=Material+Icons"
      rel="stylesheet">
    <style>
      body {
        font-family: sans-serif;  /* personal preference */
        display: flex;
        flex-flow: column nowrap;
        justify-content: flex-start;
        align-items: center;
      }
      h1 {
        font-size: 1.5em;
      }
      #main-content {
        width: 100%;
        max-width: 40em;
      }
      #main-content > div {
        padding: 1em;
        margin-bottom: 0.5em;
        background-color: rgb(150, 200, 200);
        box-shadow: 0.1em 0.1em 0.2em rgb(100, 100, 100);
        font-size: 1.2em;
      }
      #main-content div.error-message {
        background-color: rgb(200, 180, 160);
      }
      span.stat {
        font-weight: bold;
        font-size: 1.1em;
      }
      .material-icons {
        display: inline-flex;
        vertical-align: text-bottom;
      }
    </style>
  </head>
  <body>
    <h1>Quarantine Centres Dashboard</h1>

    <div id="main-content">
      {% if connected %}
        {% if has_data %}
          <div>
            <span class="material-icons">calendar_today</span>
            Quarantine data for
            <span class="stat">{{ data.date }}</span>
          </div>

          <div>
            <span class="material-icons" title="in use">outlined_flag</span>
            Quarantine units in use:
            <span class="stat">{{ data.units_in_use }}</span>
            <br>
            <span class="material-icons" title="available">checklist_rtl</span>
            Quarantine units available:
            <span class="stat">{{ data.units_available }}</span>
          </div>

          <div><span class="material-icons">insights</span>
            Highest availability is at:
            <ul class="centres">
              {% for centre in centres %}
                <li class="centre">
                  <span class="name">{{ centre.name }}</span>:
                  <span class="stat">{{ centre.units }} unit{{ centre.units | pluralize }}</span>
                </li>
              {% endfor %}
            </ul>
          </div>

          <div>
            <span class="material-icons">people</span>
            Number of persons quarantined:
            <span class="stat">{{ data.persons_quarantined }}</span>
            <br>
            <span class="material-icons" title="non-close contacts">safety_divider</span>
            Non-close contacts:
            <span class="stat">{{ data.non_close_contacts }}</span>
          </div>

          {% if data.count_consistent %}
            <div>
              <span class="material-icons">check</span>
              Count is consistent with quarantine records
            </div>
          {% else %}
            <div class="error-message">
              <span class="material-icons">warning</span>
              Warning: Count is not consistent with quarantine records
            </div>
          {% endif %}

        {# no data available #}
        {% else %}
          <div class="error-message">
            <span class="material-icons">error</span>
            Sorry, no data found within the past 7 days.
          </div>

        {# end-if has data #}
        {% endif %}

      {# not connected #}
      {% else %}
        <div class="error-message">
          <span class="material-icons">error</span>
          Sorry, something went wrong.  Please try again later.
        </div>

      {# end-if connected #}
      {% endif %}
    </div>
  </body>
</html>
  • views.py: Uses the generate_context() function from get_data.py in rendering dashboard3.html.
from django.shortcuts import render

from .libs import get_data


def dashboard(request):
    ctx = get_data.generate_context()
    return render(request, "qtine_dash/dashboard3.html", ctx)
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

<rant>

The *.py files have been formatted using Black

Learn from it, yes. Use it long-term... my opinion is no. There's a great quote from the official PEP8 definition:

A Foolish Consistency is the Hobgoblin of Little Minds

The authors of PEP8 intended for it to be a style guide and not syntactically or zealously enforced. Sometimes the computer is wrong. I've worked in a couple of professional settings where black was mandated, and I hated the shackles. Part of, as you describe,

writing the code in a professional manner

is understanding the rules and then understanding when they should be broken. Other linters - flake8, etc. - are more configurable and nuanced, and less nuclear.

</rant>

Change _generate_valid_dates so that it does not strftime. Internal data generation and formatting should be separate, and formatting should only occur at the edges of your program. Likewise, _call_api should accept a date object and not a string.

For your QUERY, factor out the common elements to a pre-initialisation statement before your if.

Do not string-concatenate your url. It's bizarre to JSON-serialize to query parameters. It's typical to see either standard RFC1738/3986-encoded query parameters as passed in a GET, or JSON as passed in a POST. For the former, pass your QUERY dict to the params kwarg of get. This assumes that you control the URL format for www.chp.gov.hk which you might not.

Don't None-fail, as in

return res.json() if res.ok else None

Instead, response.raise_for_status and handle the exception at the outer level.

Since this is a singleton:

    confines_data = _call_api("confines", date_)
    confines_data = confines_data[0]  # singleton for one date

you can just tuple-unpack:

confines_data, = _call_api("confines", date_)

Declare a language (probably en) in your HTML.

Try to separate your styles out to a separate .css file.

It's very common these days to see non-terminated HTML5 tags like <br>. I still prefer to write <br/> because, for people who are used to visually parsing XML, this is more consistent.

\$\endgroup\$

Your Answer

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

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