-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Expand mypy static type checking #32591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
55c9e47
d33a145
30b54c0
3555e28
f485794
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -201,7 +201,7 @@ def _determine_user(self, request, course_key: CourseKey) -> types.User: | |
| # Just like in masquerading, set real_user so that the | ||
| # SafeSessions middleware can see that the user didn't | ||
| # change unexpectedly. | ||
| target_user.real_user = request.user | ||
| target_user.real_user = request.user # type: ignore | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Until python typing supports intersections there is no way to tell the type system that we want to add a |
||
| return target_user | ||
|
|
||
| _course_masquerade, user = setup_masquerade(request, course_key, has_staff_access) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -268,14 +268,6 @@ class LibraryBundleLink: | |
| opaque_key = attr.ib(type=LearningContextKey, default=None) | ||
|
|
||
|
|
||
| class AccessLevel: # lint-amnesty, pylint: disable=function-redefined | ||
| """ Enum defining library access levels/permissions """ | ||
| ADMIN_LEVEL = ContentLibraryPermission.ADMIN_LEVEL | ||
| AUTHOR_LEVEL = ContentLibraryPermission.AUTHOR_LEVEL | ||
| READ_LEVEL = ContentLibraryPermission.READ_LEVEL | ||
| NO_ACCESS = None | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out this was defined twice in this file. The pylint warning was suppressed by an automatic lint amnesty line. |
||
|
|
||
|
|
||
| # General APIs | ||
| # ============ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,7 @@ def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardDa | |
| ) | ||
|
|
||
|
|
||
| def get_user_clipboard_json(user_id: int, request: HttpRequest = None): | ||
| def get_user_clipboard_json(user_id: int, request: HttpRequest | None = None): | ||
| """ | ||
| Get the detailed status of the user's clipboard, in exactly the same format | ||
| as returned from the | ||
|
|
@@ -88,7 +88,7 @@ def get_staged_content_static_files(staged_content_id: int) -> list[StagedConten | |
| sc = _StagedContent.objects.get(pk=staged_content_id) | ||
|
|
||
| def str_to_key(source_key_str: str): | ||
| if not str: | ||
| if not source_key_str: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a clear bug found by the type check. 🐛 |
||
| return None | ||
| try: | ||
| return AssetKey.from_string(source_key_str) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| """ | ||
| Data structures for the XBlock Django app's python APIs | ||
| """ | ||
| from enum import Enum | ||
|
|
||
|
|
||
| class StudentDataMode(Enum): | ||
| """ | ||
| Is student data (like which answer was selected) persisted in the DB or just stored temporarily in the session? | ||
| Generally, the LMS uses persistence and Studio uses ephemeral data. | ||
| """ | ||
| Ephemeral = 'ephemeral' | ||
| Persisted = 'persisted' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was unclear if this should be changed to allow
Noneor ifNonevalues should be converted to""when stored in this field, but there were clear examples if it being explicitly set toNonein the tests, so I went with this.https://github.com/openedx/edx-platform/blob/478c301df5ab0a58dad43cedfc05e4003828c2d3/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py#L55