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)
Reinderien
70.9k5 gold badges76 silver badges256 bronze badges
asked Mar 5, 2022 at 10:05
\$\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.

answered Mar 5, 2022 at 21:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.