Skip to content

Commit

Permalink
Enable mypy's check_untyped_defs
Browse files Browse the repository at this point in the history
Make necessary changes to allow a clean mypy run after the change
  • Loading branch information
marcospri committed Jul 12, 2024
1 parent 7859484 commit 3253aba
Show file tree
Hide file tree
Showing 23 changed files with 40 additions and 33 deletions.
4 changes: 2 additions & 2 deletions lms/models/assignment.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import sqlalchemy as sa
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.ext.mutable import MutableDict
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy.orm import DynamicMapped, Mapped, mapped_column

from lms.db import Base
from lms.models._mixins import CreatedUpdatedMixin
Expand Down Expand Up @@ -78,7 +78,7 @@ class Assignment(CreatedUpdatedMixin, Base):
deep_linking_uuid: Mapped[str | None] = mapped_column(sa.Unicode, nullable=True)
"""UUID that identifies the deep linking that created this assignment."""

groupings: Mapped[list[Grouping]] = sa.orm.relationship(
groupings: DynamicMapped[Grouping] = sa.orm.relationship(
secondary="assignment_grouping", viewonly=True, lazy="dynamic"
)

Expand Down
10 changes: 4 additions & 6 deletions lms/models/oauth2_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from enum import Enum, unique

import sqlalchemy as sa
from sqlalchemy.orm import Mapped, mapped_column

from lms.db import Base, varchar_enum

Expand Down Expand Up @@ -43,7 +44,7 @@ class OAuth2Token(Base):
sa.UniqueConstraint("user_id", "application_instance_id", "service"),
)

id = sa.Column(sa.Integer(), autoincrement=True, primary_key=True)
id: Mapped[int] = mapped_column(autoincrement=True, primary_key=True)

#: The LTI user_id of the LMS user who this access token belongs to.
user_id = sa.Column(sa.UnicodeText(), nullable=False)
Expand Down Expand Up @@ -91,11 +92,8 @@ class OAuth2Token(Base):
#: inserting new rows) then whenever `access_token` and `expires_in`
#: are updated with new values from the authorization server, `received_at`
#: should also be updated to the current time.
received_at = sa.Column(
sa.DateTime,
default=datetime.datetime.utcnow,
server_default=sa.func.now(),
nullable=False,
received_at: Mapped[datetime.datetime] = mapped_column(
default=datetime.datetime.utcnow, server_default=sa.func.now()
)

#: The API that this token is used with. In OAuth 2.0 parlance, this
Expand Down
1 change: 1 addition & 0 deletions lms/product/plugin/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# type: ignore
from lms.product.plugin.course_copy import CourseCopyPlugin
from lms.product.plugin.grouping import GroupingPlugin
from lms.product.plugin.misc import MiscPlugin
Expand Down
4 changes: 2 additions & 2 deletions lms/product/plugin/course_copy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import Callable
from typing import Any, Callable

from lms.models import File
from lms.services.exceptions import ExternalRequestError, OAuth2TokenError
Expand All @@ -20,7 +20,7 @@ def is_file_in_course(self, course_id, file_id, type_) -> bool:

def find_matching_file_in_course(
self,
store_new_course_files: Callable[[str], None],
store_new_course_files: Callable[[str], Any] | Callable[[int], Any],
file_type: str,
original_file_id,
new_course_id,
Expand Down
1 change: 1 addition & 0 deletions lms/product/plugin/plugin.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# type: ignore
from dataclasses import dataclass
from typing import cast

Expand Down
2 changes: 1 addition & 1 deletion lms/product/product.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from dataclasses import InitVar, dataclass

from lms.product.family import Family # pylint:disable=unused-import
from lms.product.plugin import PluginConfig, Plugins
from lms.product.plugin import PluginConfig, Plugins # type: ignore


@dataclass(frozen=True)
Expand Down
4 changes: 2 additions & 2 deletions lms/services/application_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ def __init__(self, application_instance: ApplicationInstance):
)


def _email_or_domain_match(columns, email):
def _email_or_domain_match(columns, email: str):
"""
Get an SQL comparator for matching emails.
This will match the full email if it contains '@' or interpret the text
as a domain if not. This will search over all the provided fields.
"""
return sa.or_(
return sa.or_( # type: ignore
(
sa.func.lower(column) == email.lower()
if "@" in email
Expand Down
5 changes: 3 additions & 2 deletions lms/services/digest.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def course_infos(self):

for row in self._db.execute(query):
# SQLAlchemy returns None instead of [].
authority_provided_ids = row.authority_provided_ids or []
row_authority_provided_ids = row.authority_provided_ids or []
instructor_h_userids = row.instructor_h_userids or []

self._course_infos.append(
Expand All @@ -411,7 +411,8 @@ def course_infos(self):
learner_annotations=tuple(
annotation
for annotation in self.annotations
if annotation.authority_provided_id in authority_provided_ids
if annotation.authority_provided_id
in row_authority_provided_ids
and annotation.userid not in instructor_h_userids
),
)
Expand Down
2 changes: 1 addition & 1 deletion lms/services/group_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def upsert_group_info(self, grouping: Grouping, params: dict):
# AI should be many:many, and we reflect that wrongness here.
group_info.application_instance = grouping.application_instance

group_info.type = self._GROUPING_TYPES[grouping.type] # type: ignore
group_info.type = self._GROUPING_TYPES[grouping.type]

for field in [
# Most (all) of these are duplicated elsewhere, we'll keep updating for now
Expand Down
4 changes: 2 additions & 2 deletions lms/services/moodle.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ def _construct_file_tree(course_id, files):
"children": [],
}
folders[component] = new_folder
current_node["children"].append(new_folder)
current_node["children"].append(new_folder) # type: ignore
current_node = folders[component]

file_node = {
Expand All @@ -266,7 +266,7 @@ def _construct_file_tree(course_id, files):
"lms_id": file_data["url"],
"updated_at": file_data["updated_at"],
}
current_node["children"].append(file_node)
current_node["children"].append(file_node) # type: ignore

return root["children"]

Expand Down
2 changes: 1 addition & 1 deletion lms/validation/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,5 @@ def wrapper_view(context, request):


def includeme(config):
_validated_view.options = ["schema"]
_validated_view.options = ["schema"] # type: ignore
config.add_view_deriver(_validated_view)
2 changes: 1 addition & 1 deletion lms/views/admin/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def assignment_dashboard(self):
"dashboard.assignment",
public_id=assignment.groupings[
0
].application_instance.organization.public_id,
].application_instance.organization.public_id, # type: ignore
assignment_id=assignment.id,
),
)
Expand Down
3 changes: 2 additions & 1 deletion lms/views/admin/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def __init__(self, request) -> None:
self.organization_service: OrganizationService = request.find_service(
OrganizationService
)
self.organization_usage_report_service: OrganizationService = (
self.organization_usage_report_service: OrganizationUsageReportService = (
request.find_service(OrganizationUsageReportService)
)

Expand Down Expand Up @@ -155,6 +155,7 @@ def toggle_organization_enabled(self):
request_org_id = self.request.matchdict["id_"]
for org_id in self.organization_service.get_hierarchy_ids(request_org_id):
org = self.organization_service.get_by_id(org_id)
assert org, "Organization {org_id} not found"
self.organization_service.update_organization(
org, enabled=self.request.params.get("enabled", "") == "on"
)
Expand Down
6 changes: 3 additions & 3 deletions lms/views/api/blackboard/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ def via_url(self):
)

document_url = self.request.params["document_url"]
file_id = course.get_mapped_file_id(
DOCUMENT_URL_REGEX.search(document_url)["file_id"] # type: ignore
)
document_url_match = DOCUMENT_URL_REGEX.search(document_url)
assert document_url_match
file_id = course.get_mapped_file_id(document_url_match["file_id"])
try:
if self.request.lti_user.is_instructor:
if not self.course_copy_plugin.is_file_in_course(course_id, file_id):
Expand Down
2 changes: 1 addition & 1 deletion lms/views/api/canvas/authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def oauth2_redirect_error(request):
)

if error_description := request.params.get("error_description"):
kwargs["error_details"] = {"error_description": error_description}
kwargs["error_details"] = {"error_description": error_description} # type: ignore

request.context.js_config.enable_oauth2_redirect_error_mode(**kwargs)

Expand Down
1 change: 1 addition & 0 deletions lms/views/api/canvas/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def via_url(self):
)

document_url_match = DOCUMENT_URL_REGEX.search(assignment.document_url)
assert document_url_match
public_url = self.canvas.public_url_for_file(
assignment,
document_url_match["file_id"],
Expand Down
1 change: 1 addition & 0 deletions lms/views/api/canvas/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ def proxy(self):
@staticmethod
def _parse_document_url(document_url):
document_url_match = DOCUMENT_URL_REGEX.search(document_url)
assert document_url_match
course_id = document_url_match["course_id"]
page_id = document_url_match["page_id"]

Expand Down
2 changes: 1 addition & 1 deletion lms/views/api/d2l/authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
permission=Permissions.API,
)
def authorize(request):
scopes = []
scopes: list[str] = []

if request.product.settings.files_enabled:
scopes += FILES_SCOPES
Expand Down
6 changes: 3 additions & 3 deletions lms/views/api/d2l/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ def via_url(_context, request):
course_id, raise_on_missing=True
)

file_id = course.get_mapped_file_id(
DOCUMENT_URL_REGEX.search(document_url)["file_id"]
)
document_url_match = DOCUMENT_URL_REGEX.search(document_url)
assert document_url_match
file_id = course.get_mapped_file_id(document_url_match["file_id"])
try:
if request.lti_user.is_instructor:
if not course_copy_plugin.is_file_in_course(course_id, file_id):
Expand Down
6 changes: 4 additions & 2 deletions lms/views/api/moodle/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ def via_url(_context, request):
)

document_url = request.params["document_url"]
document_course_id = DOCUMENT_URL_REGEX.search(document_url)["course_id"]
document_file_id = DOCUMENT_URL_REGEX.search(document_url)["url"]
document_url_match = DOCUMENT_URL_REGEX.search(document_url)
assert document_url_match
document_course_id = document_url_match["course_id"]
document_file_id = document_url_match["url"]
effective_file_id = _effective_file_id(
course_copy_plugin, course, document_url, document_course_id, document_file_id
)
Expand Down
1 change: 1 addition & 0 deletions lms/views/api/moodle/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def proxy(self):
@staticmethod
def _parse_document_url(document_url):
document_url_match = DOCUMENT_URL_REGEX.search(document_url)
assert document_url_match
course_id = document_url_match["course_id"]
page_id = document_url_match["page_id"]

Expand Down
2 changes: 1 addition & 1 deletion lms/views/lti/basic_launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def _configure_assignment(self, assignment):
)

def _configure_js_for_file_picker(
self, assignment: Assignment, route: str = "configure_assignment"
self, assignment: Assignment | None, route: str = "configure_assignment"
) -> dict:
"""
Show the file-picker for the user to choose a document.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ python_version = 3.11
warn_unused_configs = true
warn_redundant_casts = true
warn_unused_ignores = true

check_untyped_defs = true

disable_error_code = [
"import-untyped",
Expand Down

0 comments on commit 3253aba

Please sign in to comment.