Dev notes: running mypy on edx-platform

I wanted to post here some of my findings on running mypy on edx-platform. I recently added type checks to Tutor, which has allowed me to detect quite a few non-trivial bugs. Now that the Tutor code base is fully typed, I feel more confident when refactoring code or adding features. I think the Tutor code is more readable, too.

I recently noticed that some people (:eyes: @kmccormick) have started adding type annotations to edx-platform, and I think that’s great. However, mypy is not currently run as part of the edx-platform test suite. I think it would be great to do just that so I played with the mypy CLI a little. I thought it might be interesting to post here the result of running mypy on edx-platform/openedx/core:

$ mypy --follow-imports=silent --ignore-missing-imports --allow-untyped-globals --exclude=tests openedx/core/
openedx/core/lib/blockstore_api/models.py:22: error: Unsupported converter, only named functions and types are currently supported
openedx/core/lib/blockstore_api/models.py:31: error: Unsupported converter, only named functions and types are currently supported
openedx/core/lib/blockstore_api/models.py:45: error: Unsupported converter, only named functions and types are currently supported
openedx/core/lib/blockstore_api/models.py:46: error: Unsupported converter, only named functions and types are currently supported
openedx/core/lib/blockstore_api/models.py:77: error: Unsupported converter, only named functions and types are currently supported
openedx/core/djangoapps/content/learning_sequences/data.py:177: error: Need type annotation for 'course_visibility'
openedx/core/djangoapps/content/learning_sequences/data.py:305: error: Variable "openedx.core.djangoapps.content.learning_sequences.data.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/data.py:305: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/base.py:36: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.base.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/base.py:36: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/special_exams.py:61: error: User? has no attribute "id"
openedx/core/djangoapps/user_api/admin.py:117: error: "Callable[[UserRetirementStatusAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/user_api/admin.py:198: error: "Callable[[UserRetirementPartnerReportingStatusAdmin, Any, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/schedules/admin.py:53: error: "partial[Any]" has no attribute "short_description"
openedx/core/djangoapps/schedules/admin.py:54: error: "partial[Any]" has no attribute "__name__"
openedx/core/djangoapps/schedules/admin.py:139: error: "Callable[[ScheduleAdmin, Any], Any]" has no attribute "short_descriptions"
openedx/core/djangoapps/schedules/admin.py:148: error: "Callable[[ScheduleAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/schedules/admin.py:159: error: "Callable[[ScheduleAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/enrollments/serializers.py:103: error: Incompatible types in assignment (expression has type "Tuple[str, str, str, str, str, str]", base class "Meta" defined the type as "Tuple[str, str, str, str, str]")
openedx/core/djangoapps/discussions/models.py:85: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "StackedConfigurationModel" defined the type as "Tuple[str]")
openedx/core/djangoapps/discussions/models.py:194: error: Name 'cls' is not defined
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:36: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.schedule.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:36: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:39: error: Need type annotation for 'keys_to_schedule_fields'
openedx/core/djangoapps/content/learning_sequences/api/processors/content_gating.py:24: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.content_gating.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/content_gating.py:24: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/xblock/runtime/runtime.py:376: error: Name 'Callable' is not defined
openedx/core/djangoapps/xblock/runtime/runtime.py:376: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Callable")
openedx/core/djangoapps/xblock/runtime/runtime.py:376: error: Name 'UsageKey' is not defined
openedx/core/djangoapps/xblock/runtime/runtime.py:376: error: Name 'Union' is not defined
openedx/core/djangoapps/xblock/runtime/runtime.py:376: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Union")
openedx/core/djangoapps/xblock/runtime/runtime.py:376: error: Name 'ANONYMOUS_USER' is not defined
openedx/core/djangoapps/xblock/runtime/runtime.py:377: error: Name 'Union' is not defined
openedx/core/djangoapps/xblock/runtime/runtime.py:377: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Union")
openedx/core/djangoapps/xblock/runtime/runtime.py:402: error: Need type annotation for '_error_trackers' (hint: "_error_trackers: Dict[<type>, <type>] = ...")
openedx/core/djangoapps/discussions/admin.py:79: error: Incompatible types in assignment (expression has type "Tuple[str, str]", base class "StackedConfigModelAdmin" defined the type as "Tuple[str, str, str, str]")
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:233: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:233: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:250: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:250: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:272: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:272: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:285: error: User? has no attribute "id"
openedx/core/djangoapps/schedules/tasks.py:195: error: Cannot assign to a type
openedx/core/djangoapps/schedules/tasks.py:209: error: Cannot assign to a type
openedx/core/djangoapps/schedules/tasks.py:223: error: Cannot assign to a type
openedx/core/djangoapps/schedules/tasks.py:270: error: Cannot assign to a type
openedx/core/djangoapps/content/learning_sequences/admin.py:87: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:91: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:95: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:96: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:100: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:101: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:105: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:106: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:110: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:111: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:204: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:208: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:219: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:232: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:236: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:240: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py:19: error: Incompatible types in assignment (expression has type "Tuple[int]", base class "SendEmailBaseCommand" defined the type as "range")
openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py:19: error: Incompatible types in assignment (expression has type "Tuple[int, int]", base class "SendEmailBaseCommand" defined the type as "range")
openedx/core/djangoapps/content_libraries/api.py:224: error: Name 'AccessLevel' already defined on line 152
Found 56 errors in 18 files (checked 915 source files)

Thoughts?

3 Likes

I am big fan of static analysis and would love to have the extra confidence & coherence that checked annotations provide. Some edx-platform’s more, uh, “magical” code (modulestore, the xblock runtimes, etc) would be a lot more approachable if correctly annotated.

With mypy, there’s going to be some up-front configuration and refactoring to get it to pass on edx-platform. Is it possible to start by checking a smaller piece of the project, and then working out from there? The new learning_sequences app could be a good candidate, being relatively new, well-documented, and partially annotated.

If we could get even a single directory passing, I would support a PR to add mypy to the edx-platform test suite (preferably as a GitHub action).

One other thought:

I know Cale had tried getting mypy running at one point, but got stuck because of the way we hacked sys.path to allow importing directly from the djangoapps/ folders, which confused mypy (as well as import-linter, and probably a number of other static analysis tools). We’ve since fixed that.

There is still the strangeness that is the local projects in common/lib, which we install into edx-platform and then import from as if they are root packages (eg, from xmodule import ... instead of from common.lib.xmodule.xmodule import ...). I am curious whether these would be a stumbling block to getting mypy running. If they do end up being an issue, let me know, and I will add it to my list of justifications for spending the time to fix the common/lib nonsense :slight_smile:

1 Like

It’s definitely possible to run mypy on a selected list of modules; this is actually the recommended approach on an existing codebase. I have some success running mypy on the learning_sequences app, with the following mypy.ini configuration file:

[mypy]
follow_imports = silent
ignore_missing_imports = True
allow_untyped_globals = True
exclude = tests
files = openedx/core/djangoapps/content/learning_sequences/

Here are the reported errors:

$  mypy --config-file ./mypy.ini
openedx/core/djangoapps/content/learning_sequences/data.py:214: error: Need type annotation for 'course_visibility'
openedx/core/djangoapps/content/learning_sequences/data.py:342: error: Variable "openedx.core.djangoapps.content.learning_sequences.data.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/data.py:342: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/base.py:36: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.base.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/base.py:36: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/special_exams.py:61: error: User? has no attribute "id"
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:36: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.schedule.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:36: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/processors/schedule.py:39: error: Need type annotation for 'keys_to_schedule_fields'
openedx/core/djangoapps/content/learning_sequences/api/processors/content_gating.py:24: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.processors.content_gating.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/processors/content_gating.py:24: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:258: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:258: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:275: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:275: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:297: error: Variable "openedx.core.djangoapps.content.learning_sequences.api.outlines.User" is not valid as a type
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:297: note: See https://mypy.readthedocs.io/en/latest/common_issues.html#variables-vs-type-aliases
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:310: error: User? has no attribute "id"
openedx/core/djangoapps/content/learning_sequences/api/outlines.py:356: error: Argument "accessible_sequences" to "UserCourseOutlineData" has incompatible type "Set[Any]"; expected "FrozenSet[Any]"
openedx/core/djangoapps/content/learning_sequences/admin.py:87: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:91: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:95: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:96: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:100: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:101: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:105: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:106: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:110: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "boolean"
openedx/core/djangoapps/content/learning_sequences/admin.py:111: error: "Callable[[CourseSectionSequenceInline, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:204: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:208: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:219: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:232: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:236: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
openedx/core/djangoapps/content/learning_sequences/admin.py:240: error: "Callable[[CourseContextAdmin, Any], Any]" has no attribute "short_description"
Found 28 errors in 7 files (checked 33 source files)

I think I can open a PR that adds:

  1. this mypy.ini file
  2. the mypy dev dependency
  3. a “make” command that runs mypy
  4. a github action run on every PR
  5. changes to address all errors reported by mypy with this specific config

Would you be interested in that?

Sounds good to me!

@dave , do you have any qualms with Regis hacking on learning_sequences a bit in order to get mypy to pass on it?

Also, @jmbowman , does adding a required mypy check to edx-platform as a GitHub action sound good to you? I am checking with you since one of your teams would likely end up owning the mypy.ini and GitHub action files, if I had to guess :slightly_smiling_face:

I’m open to adding a mypy GitHub action as long as it’s relatively fast, works reliably, and doesn’t require developers to start cleaning up old problems while trying to get new tasks done.

I’d be delighted! Thanks @regis! This is an area that I’m definitely still learning about. :slight_smile:

I’m open to adding a mypy GitHub action as long as it’s relatively fast, works reliably, and doesn’t require developers to start cleaning up old problems while trying to get new tasks done.

I believe all these objectives can be achieved :slight_smile:

Here is the PR: https://github.com/edx/edx-platform/pull/27575

1 Like