add type hints

.github/CONTRIBUTING.md

@@ -31,34 +31,36 @@ For Python code, we generally follow [PEP 8](https://www.python.org/dev/peps/pep
 We get around Python flexible type system in several ways:
 * we try to avoid "magic" (e.g., generating or changing classes on the fly);
 * we are fairly verbose with naming, trying to help the reader with following the types;
-* we follow our type annotation system for method and function docstrings
-  (planning to switch to [PEP 484](https://www.python.org/dev/peps/pep-0484/));
-  see later for the format.
+* We use [PEP 484](https://www.python.org/dev/peps/pep-0484/) type annotations.
 
-We support Python 3 only, requiring at least version 3.9.
+We support Python 3 only. See the Installation documentation for the current
+minimum supported Python version.
 
-# Docstring type annotation format
+# Docstring format
 
-We use a custom format for type annotation in method and function docstrings. Here's an example taken from the code:
+Our docstring format is simple, if a little nonstandard.
+Here's an example taken from the code:
 
 ```
 class Cls(object):
     [...]
-    def example(self, a, b, c=None):
+    def example(
+        self, a: int, b: list[dict[str, int]], c: Submission | None = None
+    ) -> tuple[int, str]:
         """Perform an example action, described here in one line.
 
         This is a longer description of what the method does and can
         occupy more than one line, each shorter than 80 characters.
 
-        a (int): a is a required integer.
-        b ([{str: int}]): b is a list of dictionaries mapping strings to
-             integers, and note how the docstring wraps with indent.
-        c (Submission|None): c is either a Submission (not required to
-            fully specify, but it could be helpful for symbols that are
-            not imported) or None.
+        a: a is a required integer.
+        b: b is a list of dictionaries mapping strings to integers, and
+            note how the docstring wraps with indent.
+        c: c is either a Submission (not required to fully specify, but
+            it could be helpful for symbols that are not imported) or
+            None.
 
-        return ((int, str)): this method returns a tuple containing an
-            integer and a string.
+        return: this method returns a tuple containing an integer and a
+            string.
 
         raise (ValueError): if a is negative.
         raise (LookupError): if we could not find something.

cms/conf.py

@@ -26,24 +26,28 @@
 import logging
 import os
 import sys
-from collections import namedtuple
+import typing
 
-from .log import set_detailed_logs
+from cms.log import set_detailed_logs
 
 
 logger = logging.getLogger(__name__)
 
 
-class Address(namedtuple("Address", "ip port")):
+class Address(typing.NamedTuple):
+    ip: str
+    port: int
     def __repr__(self):
         return "%s:%d" % (self.ip, self.port)
 
 
-class ServiceCoord(namedtuple("ServiceCoord", "name shard")):
+class ServiceCoord(typing.NamedTuple):
     """A compact representation for the name and the shard number of a
     service (thus identifying it).
 
     """
+    name: str
+    shard: int
     def __repr__(self):
         return "%s,%d" % (self.name, self.shard)
 
@@ -67,8 +71,8 @@ class AsyncConfig:
     anyway not constantly.
 
     """
-    core_services = {}
-    other_services = {}
+    core_services: dict[ServiceCoord, Address] = {}
+    other_services: dict[ServiceCoord, Address] = {}
 
 
 async_config = AsyncConfig()
@@ -184,7 +188,7 @@ def __init__(self):
         # change the log configuration.
         set_detailed_logs(self.stream_log_detailed)
 
-    def _load(self, paths):
+    def _load(self, paths: list[str]):
         """Try to load the config files one at a time, until one loads
         correctly.
 
@@ -196,7 +200,7 @@ def _load(self, paths):
             logging.warning("No configuration file found: "
                             "falling back to default values.")
 
-    def _load_unique(self, path):
+    def _load_unique(self, path: str):
         """Populate the Config class with everything that sits inside
         the JSON file path (usually something like /etc/cms.conf). The
         only pieces of data treated differently are the elements of
@@ -206,7 +210,7 @@ def _load_unique(self, path):
         Services whose name begins with an underscore are ignored, so
         they can be commented out in the configuration file.
 
-        path (string): the path of the JSON config file.
+        path: the path of the JSON config file.
 
         """
         # Load config file.

cms/db/__init__.py

@@ -45,6 +45,7 @@
     "version", "engine",
     # session
     "Session", "ScopedSession", "SessionGen", "custom_psycopg2_connection",
+    "Session",
     # types
     "CastingArray", "Codename", "Filename", "FilenameSchema",
     "FilenameSchemaArray", "Digest",
@@ -88,7 +89,7 @@
 
 metadata = MetaData(engine)
 
-from .session import Session, ScopedSession, SessionGen, \
+from .session import Session, Session, ScopedSession, SessionGen, \
     custom_psycopg2_connection
 
 from .types import CastingArray, Codename, Filename, FilenameSchema, \
@@ -119,14 +120,14 @@
 # The following is a method of Dataset that cannot be put in the right
 # file because of circular dependencies.
 
-def get_submission_results_for_dataset(self, dataset):
+def get_submission_results_for_dataset(self, dataset) -> list[SubmissionResult]:
     """Return a list of all submission results against the specified
     dataset.
 
     Also preloads the executable and evaluation objects relative to
     the submission results.
 
-    returns ([SubmissionResult]): list of submission results.
+    returns: list of submission results.
 
     """
     # We issue this query manually to optimize it: we load all

cms/db/admin.py

@@ -23,7 +23,8 @@
 from sqlalchemy.schema import Column
 from sqlalchemy.types import Boolean, Integer, Unicode
 
-from . import Codename, Base
+from .types import Codename
+from .base import Base
 
 
 class Admin(Base):
@@ -38,46 +39,46 @@ class Admin(Base):
     __tablename__ = 'admins'
 
     # Auto increment primary key.
-    id = Column(
+    id: int = Column(
         Integer,
         primary_key=True)
 
     # Real name (human readable) of the user.
-    name = Column(
+    name: str = Column(
         Unicode,
         nullable=False)
 
     # Username used to log in in AWS.
-    username = Column(
+    username: str = Column(
         Codename,
         nullable=False,
         unique=True)
 
     # String used to authenticate the user, in the format
     # <authentication type>:<authentication_string>
-    authentication = Column(
+    authentication: str = Column(
         Unicode,
         nullable=False)
 
     # Whether the account is enabled. Disabled accounts have their
     # info kept in the database, but for all other purposes it is like
     # they did not exist.
-    enabled = Column(
+    enabled: bool = Column(
         Boolean,
         nullable=False,
         default=True)
 
     # All-access bit. If this is set, the admin can do any operation
     # in AWS, regardless of the value of the other access bits.
-    permission_all = Column(
+    permission_all: bool = Column(
         Boolean,
         nullable=False,
         default=False)
 
     # Messaging-access bit. If this is set, the admin can communicate
     # with the contestants via announcement, private messages and
     # questions.
-    permission_messaging = Column(
+    permission_messaging: bool = Column(
         Boolean,
         nullable=False,
         default=False)

cms/db/base.py

@@ -21,6 +21,7 @@
 
 import ipaddress
 from datetime import datetime, timedelta
+import typing
 
 from sqlalchemy.dialects.postgresql import ARRAY, CIDR, JSONB, OID
 from sqlalchemy.ext.declarative import as_declarative
@@ -32,6 +33,8 @@
     Boolean, Integer, Float, String, Unicode, Enum, DateTime, Interval, \
     BigInteger
 
+from cms.db.session import Session
+
 from . import engine, metadata, CastingArray, Codename, Filename, \
     FilenameSchema, FilenameSchemaArray, Digest
 
@@ -59,7 +62,8 @@
 }
 
 
-@as_declarative(bind=engine, metadata=metadata, constructor=None)
+# this has an @as_declarative, but to ease type checking it's applied manually
+# after the class definition, only when not type-checking (i.e. at runtime).
 class Base:
     """Base class for all classes managed by SQLAlchemy. Extending the
     base class given by SQLAlchemy.
@@ -187,20 +191,20 @@ def __init__(self, *args, **kwargs):
             raise
 
     @classmethod
-    def get_from_id(cls, id_, session):
+    def get_from_id(cls, id_: tuple | int | str, session: Session) -> typing.Self | None:
         """Retrieve an object from the database by its ID.
 
         Use the given session to fetch the object of this class with
         the given ID, and return it. If it doesn't exist return None.
 
-        cls (type): the class to which the method is attached.
-        id_ (tuple, int or string): the ID of the object we want; in
+        cls: the class to which the method is attached.
+        id_: the ID of the object we want; in
             general it will be a tuple (one int for each of the columns
             that make up the primary key) but if there's only one then
             a single int (even encoded as unicode or bytes) will work.
-        session (Session): the session to query.
+        session: the session to query.
 
-        return (Base|None): the desired object, or None if not found.
+        return: the desired object, or None if not found.
 
         """
         try:
@@ -213,26 +217,26 @@ def get_from_id(cls, id_, session):
         except ObjectDeletedError:
             return None
 
-    def clone(self):
+    def clone(self) -> typing.Self:
         """Copy all the column properties into a new object
 
         Create a new object of this same type and set the values of all
         its column properties to the ones of this "old" object. Leave
         the relationship properties unset.
 
-        return (object): a clone of this object
+        return: a clone of this object
 
         """
         cls = type(self)
         args = list(getattr(self, prp.key) for prp in self._col_props)
         return cls(*args)
 
-    def get_attrs(self):
+    def get_attrs(self) -> dict[str, object]:
         """Return self.__dict__.
 
         Limited to SQLAlchemy column properties.
 
-        return ({string: object}): the properties of this object.
+        return: the properties of this object.
 
         """
         attrs = dict()
@@ -241,14 +245,14 @@ def get_attrs(self):
                 attrs[prp.key] = getattr(self, prp.key)
         return attrs
 
-    def set_attrs(self, attrs, fill_with_defaults=False):
+    def set_attrs(self, attrs: typing.Mapping[str, object], fill_with_defaults: bool = False):
         """Do self.__dict__.update(attrs) with validation.
 
         Limited to SQLAlchemy column and relationship properties.
 
-        attrs ({string: object}): the new properties we want to set on
+        attrs: the new properties we want to set on
             this object.
-        fill_with_defaults (bool): whether to explicitly reset the
+        fill_with_defaults: whether to explicitly reset the
             attributes that were not provided in attrs to their default
             value.
 
@@ -318,3 +322,9 @@ def set_attrs(self, attrs, fill_with_defaults=False):
                 "set_attrs() got an unexpected keyword argument '%s'" %
                 attrs.popitem()[0])
 
+# don't apply the decorator when type checking, as as_declarative doesn't have
+# enough type hints for pyright to consider it valid. This means that pyright
+# doesn't consider Base to be a valid base class, and thus all derived classes
+# will be missing the methods from Base.
+if not typing.TYPE_CHECKING:
+    Base = as_declarative(bind=engine, metadata=metadata, constructor=None)(Base)