Skip to content

Commit

Permalink
feat: run mypy as part of testing the codebase
Browse files Browse the repository at this point in the history
The edx-platform codebase already includes quite a few type annotations, but
they were not regularly checked. This may cause problems, when the annotations
themselves include errors (as we found out in some of the learning_sequences
annotations). So here, we add mypy as a dev requirement and introduce a make
command to run mypy regularly. Mypy runs on a very small portion of the total
edx-platform, as configured in mypy.ini. Our hope is that developers will add
more and more modules to this configuration file, until we can eventually run
mypy on the full code base.

See discussion: https://discuss.openedx.org/t/dev-notes-running-mypy-on-edx-platform/4860
  • Loading branch information
regisb committed May 27, 2021
1 parent b6fe11e commit 88a2d28
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 48 deletions.
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ compile-requirements: ## Re-compile *.in requirements to *.txt
upgrade: pre-requirements ## update the pip requirements files to use the latest releases satisfying our constraints
$(MAKE) compile-requirements COMPILE_OPTS="--upgrade"

check-types: ## run static type-checking tests
mypy

# These make targets currently only build LMS images.
docker_build:
docker build . -f Dockerfile --target lms -t openedx/edx-platform
Expand Down
6 changes: 6 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[mypy]
follow_imports = silent
ignore_missing_imports = True
allow_untyped_globals = True
exclude = tests
files = openedx/core/djangoapps/content/learning_sequences/,openedx/core/types
29 changes: 13 additions & 16 deletions openedx/core/djangoapps/content/learning_sequences/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from opaque_keys import OpaqueKey
import attr

from openedx.core import types
from .api import get_content_errors, get_course_outline
from .models import CourseContext, CourseSectionSequence

Expand Down Expand Up @@ -82,33 +83,29 @@ def get_queryset(self, request):
qs = qs.select_related('section', 'sequence')
return qs

@types.admin_display(description="Title")
def title(self, cs_seq):
return cs_seq.sequence.title
title.short_description = "Title"

@types.admin_display(boolean=True)
def accessible_after_due(self, cs_seq):
return not cs_seq.inaccessible_after_due
accessible_after_due.boolean = True

@types.admin_display(boolean=True, description="Staff Only")
def visible_to_staff_only(self, cs_seq):
return not cs_seq.visible_to_staff_only
visible_to_staff_only.boolean = True
visible_to_staff_only.short_description = "Staff Only"

@types.admin_display(boolean=True, description="Timed Exam")
def is_time_limited(self, cs_seq):
return cs_seq.exam.is_time_limited
is_time_limited.boolean = True
is_time_limited.short_description = "Timed Exam"

@types.admin_display(boolean=True, description="Proctored Exam")
def is_proctored_enabled(self, cs_seq):
return cs_seq.exam.is_proctored_enabled
is_proctored_enabled.boolean = True
is_proctored_enabled.short_description = "Proctored Exam"

@types.admin_display(boolean=True, description="Practice Exam")
def is_practice_exam(self, cs_seq):
return cs_seq.exam.is_practice_exam
is_practice_exam.boolean = True
is_practice_exam.short_description = "Practice Exam"


class CourseContextAdmin(admin.ModelAdmin):
Expand Down Expand Up @@ -199,24 +196,24 @@ def get_queryset(self, request):
qs = qs.prefetch_related('section_sequences')
return qs

@types.admin_display(description="Record Created")
def created(self, course_context):
return course_context.learning_context.created
created.short_description = "Record Created"

@types.admin_display(description="Record Modified")
def modified(self, course_context):
return course_context.learning_context.modified
modified.short_description = "Record Modified"

def course_key(self, course_context):
return course_context.learning_context.context_key

def title(self, course_context):
return course_context.learning_context.title

@types.admin_display(description="Published at (UTC)")
def published_at(self, course_context):
published_at_dt = course_context.learning_context.published_at
return published_at_dt.strftime("%B %-d, %Y, %-I:%M %p")
published_at.short_description = "Published at (UTC)"

def published_version(self, course_context):
return course_context.learning_context.published_version
Expand All @@ -227,17 +224,17 @@ def _publish_report_attr(self, course_context, attr_name):
return None
return getattr(learning_context.publish_report, attr_name)

@types.admin_display(description="Errors")
def num_errors(self, course_context):
return self._publish_report_attr(course_context, 'num_errors')
num_errors.short_description = "Errors"

@types.admin_display(description="Sections")
def num_sections(self, course_context):
return self._publish_report_attr(course_context, 'num_sections')
num_sections.short_description = "Sections"

@types.admin_display(description="Sequences")
def num_sequences(self, course_context):
return self._publish_report_attr(course_context, 'num_sequences')
num_sequences.short_description = "Sequences"

def raw_outline(self, obj):
"""
Expand Down
31 changes: 15 additions & 16 deletions openedx/core/djangoapps/content/learning_sequences/api/outlines.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
import logging
from collections import defaultdict
from datetime import datetime
from typing import Dict, FrozenSet, Optional, List, Union
from django.contrib.auth import get_user_model
from django.db.models.query import QuerySet
from typing import Dict, FrozenSet, List, Optional, Union

from django.db import transaction
from django.db.models.query import QuerySet
from edx_django_utils.cache import TieredCache
from edx_django_utils.monitoring import function_trace, set_custom_attribute
from opaque_keys import OpaqueKey
from opaque_keys.edx.locator import LibraryLocator
from edx_django_utils.cache import TieredCache
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import LibraryLocator
from openedx.core import types

from ..data import (
ContentErrorData,
Expand All @@ -25,30 +26,28 @@
ExamData,
UserCourseOutlineData,
UserCourseOutlineDetailsData,
VisibilityData,

VisibilityData
)
from ..models import (
ContentError,
CourseContext,
CourseSection,
CourseSectionSequence,
CourseContext,
CourseSequenceExam,
LearningContext,
LearningSequence,
PublishReport,
UserPartitionGroup,
UserPartitionGroup
)
from .permissions import can_see_all_content
from .processors.content_gating import ContentGatingOutlineProcessor
from .processors.enrollment import EnrollmentOutlineProcessor
from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor
from .processors.milestones import MilestonesOutlineProcessor
from .processors.schedule import ScheduleOutlineProcessor
from .processors.special_exams import SpecialExamsOutlineProcessor
from .processors.visibility import VisibilityOutlineProcessor
from .processors.enrollment import EnrollmentOutlineProcessor
from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor

User = get_user_model()
log = logging.getLogger(__name__)

# Public API...
Expand Down Expand Up @@ -255,7 +254,7 @@ def get_content_errors(course_key: CourseKey) -> List[ContentErrorData]:

@function_trace('learning_sequences.api.get_user_course_outline')
def get_user_course_outline(course_key: CourseKey,
user: User,
user: types.User,
at_time: datetime) -> UserCourseOutlineData:
"""
Get an outline customized for a particular user at a particular time.
Expand All @@ -272,7 +271,7 @@ def get_user_course_outline(course_key: CourseKey,

@function_trace('learning_sequences.api.get_user_course_outline_details')
def get_user_course_outline_details(course_key: CourseKey,
user: User,
user: types.User,
at_time: datetime) -> UserCourseOutlineDetailsData:
"""
Get an outline with supplementary data like scheduling information.
Expand All @@ -299,7 +298,7 @@ def get_user_course_outline_details(course_key: CourseKey,


def _get_user_course_outline_and_processors(course_key: CourseKey, # lint-amnesty, pylint: disable=missing-function-docstring
user: User,
user: types.User,
at_time: datetime):
"""
Helper function that runs the outline processors.
Expand Down Expand Up @@ -352,7 +351,7 @@ def _get_user_course_outline_and_processors(course_key: CourseKey, # lint-amnes

# Open question: Does it make sense to remove a Section if it has no Sequences in it?
trimmed_course_outline = full_course_outline.remove(usage_keys_to_remove)
accessible_sequences = set(trimmed_course_outline.sequences) - inaccessible_sequences
accessible_sequences = frozenset(set(trimmed_course_outline.sequences) - inaccessible_sequences)

user_course_outline = UserCourseOutlineData(
base_outline=full_course_outline,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
import logging
from datetime import datetime

from django.contrib.auth import get_user_model
from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=unused-import
from openedx.core import types

User = get_user_model()
log = logging.getLogger(__name__)


Expand All @@ -33,7 +32,8 @@ class OutlineProcessor:
additional methods to return specific metadata to feed into
UserCourseOutlineDetailsData.
"""
def __init__(self, course_key: CourseKey, user: User, at_time: datetime):

def __init__(self, course_key: CourseKey, user: types.User, at_time: datetime):
"""
Basic initialization.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
import logging
from datetime import datetime

from django.contrib.auth import get_user_model
from opaque_keys.edx.keys import CourseKey
from openedx.core import types

from common.djangoapps.student.models import EntranceExamConfiguration
from common.djangoapps.util import milestones_helpers

from .base import OutlineProcessor

User = get_user_model()
log = logging.getLogger(__name__)


Expand All @@ -21,7 +21,8 @@ class ContentGatingOutlineProcessor(OutlineProcessor):
- Entrance Exams
- Chapter gated content
"""
def __init__(self, course_key: CourseKey, user: User, at_time: datetime):

def __init__(self, course_key: CourseKey, user: types.User, at_time: datetime):
super().__init__(course_key, user, at_time)
self.required_content = None
self.can_skip_entrance_exam = False
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@
import logging
from collections import defaultdict # lint-amnesty, pylint: disable=unused-import
from datetime import datetime, timedelta
from typing import Dict

from django.contrib.auth import get_user_model
from edx_when.api import get_dates_for_course
from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=unused-import
from openedx.core import types

from common.djangoapps.student.auth import user_has_role
from common.djangoapps.student.roles import CourseBetaTesterRole

from ...data import ScheduleData, ScheduleItemData, UserCourseOutlineData
from .base import OutlineProcessor

User = get_user_model()
log = logging.getLogger(__name__)


Expand All @@ -33,10 +34,10 @@ class ScheduleOutlineProcessor(OutlineProcessor):
* Things that are made inaccessible after they're due.
"""

def __init__(self, course_key: CourseKey, user: User, at_time: datetime):
def __init__(self, course_key: CourseKey, user: types.User, at_time: datetime):
super().__init__(course_key, user, at_time)
self.dates = None
self.keys_to_schedule_fields = defaultdict(dict)
self.keys_to_schedule_fields: Dict[str, Dict[str, datetime]] = defaultdict(dict)
self._course_start = None
self._course_end = None
self._is_beta_tester = False
Expand Down
7 changes: 3 additions & 4 deletions openedx/core/djangoapps/content/learning_sequences/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
from typing import Dict, FrozenSet, List, Optional

import attr
from django.contrib.auth import get_user_model
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core import types

User = get_user_model()
log = logging.getLogger(__name__)


Expand Down Expand Up @@ -211,7 +210,7 @@ def not_deprecated(self, _attribute, value):
# derived from what you pass into `sections`. Do not set this directly.
sequences = attr.ib(type=Dict[UsageKey, CourseLearningSequenceData], init=False)

course_visibility = attr.ib(validator=attr.validators.in_(CourseVisibility))
course_visibility: CourseVisibility = attr.ib(validator=attr.validators.in_(CourseVisibility))

# Entrance Exam ID
entrance_exam_id = attr.ib(type=str)
Expand Down Expand Up @@ -339,7 +338,7 @@ class is a pretty dumb container that doesn't understand anything about how

# Django User representing who we've customized this outline for. This may
# be the AnonymousUser.
user: User
user: types.User

# UTC TZ time representing the time for which this user course outline was
# created. It is possible to create UserCourseOutlineData for a time in the
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ class can be used with many different XBlocks, whereas each XBlock gets its

def __init__(
self,
handler_url, # type: (Callable[[UsageKey, str, Union[int, ANONYMOUS_USER]], str]
handler_url, # type: Callable[[UsageKey, str, Union[int, ANONYMOUS_USER]], str]
student_data_mode, # type: Union[STUDENT_DATA_EPHEMERAL, STUDENT_DATA_PERSISTED]
runtime_class, # type: XBlockRuntime
):
Expand Down
5 changes: 5 additions & 0 deletions openedx/core/types/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
"""
Add here typing utilities API functions and classes.
"""
from .admin import admin_display
from .user import User
57 changes: 57 additions & 0 deletions openedx/core/types/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""
Typing utilities for the admin sites.
"""
import warnings

from typing import Any, Callable, Optional, Protocol


class AdminMethod(Protocol):
"""
Duck-type definition of a callable admin method.
See:
https://github.com/python/mypy/issues/2087#issuecomment-462726600
https://mypy.readthedocs.io/en/stable/protocols.html
https://www.python.org/dev/peps/pep-0544/
"""

short_description: str
boolean: bool


def _admin_display(
boolean: Optional[bool] = None, description: Optional[str] = None
) -> Callable[[Any], AdminMethod]:
"""
Decorator for functions that need to be annotated with attributes from AdminMethod.
This method and the above AdminMethod class will no longer be necessary in Django 3.2,
when `admin.display` is introduced:
https://docs.djangoproject.com/en/3.2/ref/contrib/admin/#django.contrib.admin.display
"""

def decorator(func: Any) -> AdminMethod:
if boolean is not None:
func.boolean = boolean
if description is not None:
func.short_description = description
return func

return decorator


try:
import django.contrib.admin

admin_display = django.contrib.admin.display
if _admin_display or AdminMethod:
warnings.warn(
(
"Django 3.2+ available: the _admin_display method and the AdminMethod"
"class should be removed from openedx.core.types"
),
DeprecationWarning,
)
except AttributeError:
admin_display = _admin_display
Loading

0 comments on commit 88a2d28

Please sign in to comment.