Add type hints and mypy

Author: mxmeinhold

.github/workflows/python-app.yml

@@ -32,3 +32,27 @@ jobs:
       - name: Lint with pylint
         run: |
           pylint packet
+
+  typecheck:
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        python-version: [3.9]
+
+    steps:
+      - name: Install ldap dependencies
+        run: sudo apt-get update && sudo apt-get install libldap2-dev libsasl2-dev
+      - uses: actions/checkout@v2
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v2
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+      - name: Typecheck with mypy
+        run: |
+          # Disabled error codes to discard errors from imports
+          mypy --disable-error-code import --disable-error-code name-defined --disallow-untyped-defs --exclude routes packet

README.md

@@ -115,13 +115,14 @@ All DB commands are from the `Flask-Migrate` library and are used to configure D
 docs [here](https://flask-migrate.readthedocs.io/en/latest/) for details. 
 
 ## Code standards
-This project is configured to use Pylint. Commits will be pylinted by GitHub actions and if the score drops your build will 
-fail blocking you from merging. To make your life easier just run it before making a PR.
+This project is configured to use Pylint and mypy. Commits will be pylinted and typechecked by GitHub actions and if the
+score drops your build will fail blocking you from merging. To make your life easier just run it before making a PR.
 
-To run pylint use this command:
+To run pylint and mypy use these commands:
 ```bash
 pylint packet/routes packet
+mypy --disable-error-code import --disable-error-code name-defined --disallow-untyped-defs --exclude routes packet
 ```
 
 All python files should have a top-level docstring explaining the contents of the file and complex functions should 
-have docstrings explaining any non-obvious portions.
+have docstrings explaining any non-obvious portions. Functions should have type annotations.

packet/__init__.py

@@ -21,7 +21,7 @@
 
 from .git import get_version
 
-app = Flask(__name__)
+app: Flask = Flask(__name__)
 gzip = Gzip(app)
 
 # Load default configuration and any environment variable overrides

packet/commands.py

@@ -5,7 +5,7 @@
 import sys
 
 from secrets import token_hex
-from datetime import datetime, time
+from datetime import datetime, time, date
 import csv
 import click
 
@@ -15,7 +15,7 @@
 
 
 @app.cli.command('create-secret')
-def create_secret():
+def create_secret() -> None:
     """
     Generates a securely random token. Useful for creating a value for use in the "SECRET_KEY" config setting.
     """
@@ -28,13 +28,13 @@ def create_secret():
 
 
 class CSVFreshman:
-    def __init__(self, row):
+    def __init__(self, row: list[str]) -> None:
         self.name = row[0].strip()
         self.rit_username = row[3].strip()
         self.onfloor = row[1].strip() == 'TRUE'
 
 
-def parse_csv(freshmen_csv):
+def parse_csv(freshmen_csv: str) -> dict[str, CSVFreshman]:
     print('Parsing file...')
     try:
         with open(freshmen_csv, newline='') as freshmen_csv_file:
@@ -44,7 +44,7 @@ def parse_csv(freshmen_csv):
         raise e
 
 
-def input_date(prompt):
+def input_date(prompt: str) -> date:
     while True:
         try:
             date_str = input(prompt + ' (format: MM/DD/YYYY): ')
@@ -55,7 +55,7 @@ def input_date(prompt):
 
 @app.cli.command('sync-freshmen')
 @click.argument('freshmen_csv')
-def sync_freshmen(freshmen_csv):
+def sync_freshmen(freshmen_csv: str) -> None:
     """
     Updates the freshmen entries in the DB to match the given CSV.
     """
@@ -68,7 +68,7 @@ def sync_freshmen(freshmen_csv):
 
 @app.cli.command('create-packets')
 @click.argument('freshmen_csv')
-def create_packets(freshmen_csv):
+def create_packets(freshmen_csv: str) -> None:
     """
     Creates a new packet season for each of the freshmen in the given CSV.
     """
@@ -84,7 +84,7 @@ def create_packets(freshmen_csv):
 
 
 @app.cli.command('ldap-sync')
-def ldap_sync():
+def ldap_sync() -> None:
     """
     Updates the upper and misc sigs in the DB to match ldap.
     """
@@ -97,7 +97,7 @@ def ldap_sync():
         help='The file to write to. If no file provided, output is sent to stdout.')
 @click.option('--csv/--no-csv', 'use_csv', required=False, default=False, help='Format output as comma separated list.')
 @click.option('--date', 'date_str', required=False, default='', help='Packet end date in the format MM/DD/YYYY.')
-def fetch_results(file_path, use_csv, date_str):
+def fetch_results(file_path: str, use_csv: bool, date_str: str) -> None:
     """
     Fetches and prints the results from a given packet season.
     """
@@ -150,7 +150,7 @@ def fetch_results(file_path, use_csv, date_str):
 
 @app.cli.command('extend-packet')
 @click.argument('packet_id')
-def extend_packet(packet_id):
+def extend_packet(packet_id: int) -> None:
     """
     Extends the given packet by setting a new end date.
     """
@@ -168,7 +168,7 @@ def extend_packet(packet_id):
     print('Packet successfully extended')
 
 
-def remove_sig(packet_id, username, is_member):
+def remove_sig(packet_id: int, username: str, is_member: bool) -> None:
     packet = Packet.by_id(packet_id)
 
     if not packet.is_open():
@@ -200,7 +200,7 @@ def remove_sig(packet_id, username, is_member):
 @app.cli.command('remove-member-sig')
 @click.argument('packet_id')
 @click.argument('member')
-def remove_member_sig(packet_id, member):
+def remove_member_sig(packet_id: int, member: str) -> None:
     """
     Removes the given member's signature from the given packet.
     :param member: The member's CSH username
@@ -211,7 +211,7 @@ def remove_member_sig(packet_id, member):
 @app.cli.command('remove-freshman-sig')
 @click.argument('packet_id')
 @click.argument('freshman')
-def remove_freshman_sig(packet_id, freshman):
+def remove_freshman_sig(packet_id: int, freshman: str) -> None:
     """
     Removes the given freshman's signature from the given packet.
     :param freshman: The freshman's RIT username

packet/context_processors.py

@@ -5,22 +5,23 @@
 import urllib
 from functools import lru_cache
 from datetime import datetime
+from typing import Callable
 
-from packet.models import Freshman
+from packet.models import Freshman, UpperSignature
 from packet import app, ldap
 
 
 # pylint: disable=bare-except
 @lru_cache(maxsize=128)
-def get_csh_name(username):
+def get_csh_name(username: str) -> str:
     try:
         member = ldap.get_member(username)
         return member.cn + ' (' + member.uid + ')'
     except:
         return username
 
 
-def get_roles(sig):
+def get_roles(sig: UpperSignature) -> dict[str, str]:
     """
     Converts a signature's role fields to a dict for ease of access.
     :return: A dictionary of role short names to role long names
@@ -45,7 +46,7 @@ def get_roles(sig):
 
 # pylint: disable=bare-except
 @lru_cache(maxsize=256)
-def get_rit_name(username):
+def get_rit_name(username: str) -> str:
     try:
         freshman = Freshman.query.filter_by(rit_username=username).first()
         return freshman.name + ' (' + username + ')'
@@ -55,7 +56,7 @@ def get_rit_name(username):
 
 # pylint: disable=bare-except
 @lru_cache(maxsize=256)
-def get_rit_image(username):
+def get_rit_image(username: str) -> str:
     if username:
         addresses = [username + '@rit.edu', username + '@g.rit.edu']
         for addr in addresses:
@@ -69,15 +70,15 @@ def get_rit_image(username):
     return 'https://www.gravatar.com/avatar/freshmen?d=mp&f=y'
 
 
-def log_time(label):
+def log_time(label: str) -> None:
     """
     Used during debugging to log timestamps while rendering templates
     """
     print(label, datetime.now())
 
 
 @app.context_processor
-def utility_processor():
+def utility_processor() -> dict[str, Callable]:
     return dict(
         get_csh_name=get_csh_name, get_rit_name=get_rit_name, get_rit_image=get_rit_image, log_time=log_time,
         get_roles=get_roles

packet/git.py

@@ -2,7 +2,7 @@
 import os
 import subprocess
 
-def get_short_sha(commit_ish: str = 'HEAD'):
+def get_short_sha(commit_ish: str = 'HEAD') -> str:
     """
     Get the short hash of a commit-ish
     Returns '' if unfound
@@ -14,7 +14,7 @@ def get_short_sha(commit_ish: str = 'HEAD'):
     except subprocess.CalledProcessError:
         return ''
 
-def get_tag(commit_ish: str = 'HEAD'):
+def get_tag(commit_ish: str = 'HEAD') -> str:
     """
     Get the name of the tag at a given commit-ish
     Returns '' if untagged
@@ -26,7 +26,7 @@ def get_tag(commit_ish: str = 'HEAD'):
     except subprocess.CalledProcessError:
         return ''
 
-def get_version(commit_ish: str = 'HEAD'):
+def get_version(commit_ish: str = 'HEAD') -> str:
     """
     Get the version string of a commit-ish