Fix: mypy failures.

Author: bjones1

bookserver/internal/utils.py

@@ -31,8 +31,8 @@ def canonicalize_tz(tstring: str) -> str:
     """
     x = re.search(r"\((.*)\)", tstring)
     if x:
-        x = x.group(1)
-        y = x.split()
+        z = x.group(1)
+        y = z.split()
         if len(y) == 1:
             return tstring
         else:

bookserver/routers/assessment.py

@@ -15,18 +15,16 @@
 #
 # Standard library
 # ----------------
-import datetime
+# None.
 
 # Third-party imports
 # -------------------
-from dateutil.parser import parse
 from fastapi import APIRouter
 
 # Local application imports
 # -------------------------
 from ..applogger import rslogger
 from ..crud import create_useinfo_entry, fetch_last_answer_table_entry  # noqa F401
-from ..internal.utils import canonicalize_tz
 from ..schemas import AssessmentRequest, LogItem, LogItemIncoming  # noqa F401
 
 # Routing
@@ -49,20 +47,6 @@ async def get_assessment_results(request_data: AssessmentRequest):
     # else:
     #     sid = auth.user.username
 
-    # :index:`todo`: **This whole thing is messy - get the deadline from the assignment in the db.**
-    if request_data.deadline:
-        try:
-            deadline = parse(canonicalize_tz(request_data.deadline))
-            tzoff = session.timezoneoffset if session.timezoneoffset else 0
-            deadline = deadline + datetime.timedelta(hours=float(tzoff))
-            deadline = deadline.replace(tzinfo=None)
-        except Exception:
-            # TODO: can this enclose just the parse code? Or can an error be raised in other cases?
-            rslogger.error(f"Bad Timezone - {request_data.deadline}")
-            deadline = datetime.datetime.utcnow()
-    else:
-        request_data.deadline = datetime.datetime.utcnow()
-
     # Identify the correct event and query the database so we can load it from the server
 
     row = await fetch_last_answer_table_entry(request_data)

bookserver/routers/rslogging.py

@@ -36,7 +36,7 @@
 # log_book_event endpoint
 # -----------------------
 @router.post("/bookevent")
-async def log_book_event(entry: LogItemIncoming):
+async def log_book_event(entry: LogItem):
     """
     This endpoint is called to log information for nearly every click that happens in the textbook.
     It uses the ``LogItem`` object to define the JSON payload it gets from a page of a book.

bookserver/schemas.py

@@ -10,16 +10,18 @@
 #
 # Standard library
 # ----------------
-from datetime import datetime
-from typing import Container, Optional, Type
+from datetime import datetime, timedelta
+from dateutil.parser import parse
+from typing import Container, Optional, Type, Dict, Tuple, Union, Any
 
 # Third-party imports
 # -------------------
-from pydantic import BaseModel, create_model, constr
+from pydantic import BaseModel, create_model, constr, validator, Field
 
 # Local application imports
 # -------------------------
 from . import models
+from .internal.utils import canonicalize_tz
 
 
 # Schema generation
@@ -36,13 +38,13 @@ def sqlalchemy_to_pydantic(
     # The base class from which the Pydantic model will inherit.
     base: Optional[Type] = None,
     # SQLAlchemy fields to exclude from the resulting schema, provided as a sequence of field names.
-    exclude: Container[str] = []
+    exclude: Container[str] = [],
 ) -> Type[BaseModel]:
 
     # If provided an ORM model, get the underlying Table object.
     db_model = getattr(db_model, "__table__", db_model)
 
-    fields = {}
+    fields: Dict[str, Union[Tuple[str, Any], Type]] = {}
     for column in db_model.columns:
         # Determine the name of this column.
         name = column.key
@@ -84,8 +86,6 @@ class LogItemIncoming(BaseModel):
     """
 
     # FIXME: Use max lengths for strings based on the actual lengths from the database using `Pydantic constraints <https://pydantic-docs.helpmanual.io/usage/types/#constrained-types>`_. Is there any way to query the database for these, instead of manually keeping them in sync?
-    #
-    # Even better, we should introspect the database and auto-convert this to Pydantic. See the idea at https://github.com/tiangolo/pydantic-sqlalchemy/blob/master/pydantic_sqlalchemy/main.py. See also https://github.com/kolypto/py-sa2schema. Other ideas: https://docs.sqlalchemy.org/en/14/core/reflection.html,
     event: str
     act: str
     div_id: str
@@ -107,19 +107,30 @@ class LogItem(LogItemIncoming):
     This may seem like overkill but it illustrates a point.  The schema for the incoming log data will not contain a timestamp.  We could make it optional there, but then that would imply that it is optional which it most certainly is not.  We could add timestamp as part of a LogItemCreate class similar to how password is handled in the tutorial: https://fastapi.tiangolo.com/tutorial/sql-databases/ But there is no security reason to exclude timestamp.  So I think this is a reasonable compromise.
     """
 
-    timestamp: datetime
+    timestamp: datetime = Field(default_factory=datetime.utcnow)
 
-    class Config:
-        orm_mode = True
-        # this tells pydantic to try read anything with attributes we give it as a model
-        # TODO: This doesn't make sense to me -- we're not using an ORM. What does this do?
+    @validator("timestamp")
+    def str_to_datetime(cls, value: str) -> datetime:
+        # TODO: this code probably doesn't work.
+        try:
+            deadline = parse(canonicalize_tz(value))
+            # TODO: session isn't defined. Here's a temporary fix
+            # tzoff = session.timezoneoffset if session.timezoneoffset else 0
+            tzoff = 0
+            deadline = deadline + timedelta(hours=float(tzoff))
+            deadline = deadline.replace(tzinfo=None)
+        except Exception:
+            # TODO: can this enclose just the parse code? Or can an error be raised in other cases?
+            raise ValueError(f"Bad Timezone - {value}")
+        return deadline
 
 
 class AssessmentRequest(BaseModel):
     course: str
     div_id: str
     event: str
     sid: Optional[str] = None
+    # See `Field with dynamic default value <https://pydantic-docs.helpmanual.io/usage/models/#required-optional-fields>`_.
     deadline: Optional[str] = None
 
 

bookserver/session.py

@@ -8,9 +8,22 @@
 # OR in a header to be validated.  If the token is valid then the user will be looked
 # up in the database using the ``load_user`` function in this file.
 # see `./routers/auth.py` for more detail.
+
+# Imports
+# =======
+# These are listed in the order prescribed by `PEP 8`_.
 #
-from bookserver.config import settings
+# Standard library
+# ----------------
+from typing import Optional
+
+# Third-party imports
+# -------------------
 from fastapi_login import LoginManager
+
+# Local application imports
+# -------------------------
+from .config import settings
 from . import schemas
 from .crud import fetch_user
 from .applogger import rslogger
@@ -21,7 +34,7 @@
 
 
 @auth_manager.user_loader
-async def load_user(user_id: str) -> schemas.User:
+async def load_user(user_id: str) -> Optional[schemas.User]:
     """
     fetch a user object from the database. This is designed to work with the
     original web2py auth_user schema but make it easier to migrate to a new

conf.py

@@ -345,7 +345,7 @@ def excluder(app: Sphinx, config: Config):
         # The path must start in the `srcdir <https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx.application.Sphinx.srcdir>`_.
         root_path = Path(app.srcdir)
         # This is slightly inefficient, since it doesn't use the existing excludes to avoid searching already-excluded values.
-        app.config.exclude_patterns += [
+        app.config.exclude_patterns += [  # type: ignore
             # Paths must be relative to the srcdir.
             x.relative_to(root_path).as_posix()
             for x in root_path.glob(pattern)

mypy.ini

@@ -26,7 +26,7 @@
 [mypy]
 # See `files <https://mypy.readthedocs.io/en/stable/config_file.html#confval-files>`_.
 files = .
-exclude = ^_build/
+exclude = (^_build/)|(^alembic/)
 
 ; These libraries lack annotations. `Ignore missing imports <https://mypy.readthedocs.io/en/latest/config_file.html#import-discovery>`_.
 [mypy-alembic.*]
@@ -35,5 +35,14 @@ ignore_missing_imports = True
 [mypy-CodeChat.*]
 ignore_missing_imports = True
 
+[mypy-fastapi_login.*]
+ignore_missing_imports = True
+
+[mypy-pydal.validators.*]
+ignore_missing_imports = True
+
 [mypy-sqlalchemy.*]
 ignore_missing_imports = True
+
+[mypy-uvicorn.*]
+ignore_missing_imports = True

pre_commit_check.py

@@ -34,7 +34,7 @@ def checks():
         "black --check .",
         # Do this next -- it should be easy to fix most of these.
         "flake8 .",
-        ##"mypy",
+        "mypy",
         # Next, check the docs. Again, these only require fixes to comments, and should still be relatively easy to correct.
         #
         # Force a `full build <https://www.sphinx-doc.org/en/master/man/sphinx-build.html>`_: