Video Block quality fixes

xblocks_contrib/html/html.py

@@ -322,7 +322,7 @@ def get_context(self):
         # the root /c4x/ url for assets. This allows client-side substitutions to occur.
         return {
             "module": self,
-            "editable_metadata_fields": self.editable_metadata_fields,  # pylint: disable=no-member
+            "editable_metadata_fields": self.editable_metadata_fields,
             "data": self.data,
             "base_asset_url": self.get_base_url_path_for_course_assets(self.location.course_key),
             "enable_latex_compiler": self.use_latex_compiler,
@@ -371,7 +371,7 @@ def bind_for_student(self, user_id, wrappers=None):
         if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id:
             if getattr(self.runtime, "position", None):
                 # update the position of the tab
-                self.position = self.runtime.position  # pylint: disable=attribute-defined-outside-init
+                self.position = self.runtime.position
             return
 
         # # If we are switching users mid-request, save the data from the old user.
@@ -402,7 +402,7 @@ def bind_for_student(self, user_id, wrappers=None):
             wrapped_field_data = self.runtime.service(self, "field-data-unbound")
             for wrapper in wrappers:
                 wrapped_field_data = wrapper(wrapped_field_data)
-            self._bound_field_data = wrapped_field_data  # pylint: disable=attribute-defined-outside-init
+            self._bound_field_data = wrapped_field_data
             if getattr(self.runtime, "uses_deprecated_field_data", False):
                 # This approach is deprecated but old mongo's CachingDescriptorSystem still requires it.
                 # For Split mongo's CachingDescriptor system, don't set ._field_data this way.

xblocks_contrib/video/content.py

@@ -1,13 +1,24 @@
-""" Video Block Static Content, class copied from StaticContent class in edx-platform/xmodule/contentstore/content.py """
+"""
+Video Block Static Content.
+
+Class copied from StaticContent class in edx-platform/xmodule/contentstore/content.py
+"""
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import AssetKey
 from opaque_keys.edx.locator import AssetLocator
 
-class VideoBlockStaticContent:  # lint-amnesty, pylint: disable=missing-class-docstring
-    def __init__(self, loc, name, content_type, data, last_modified_at=None, thumbnail_location=None, import_path=None,
-                 length=None, locked=False, content_digest=None):
+
+class VideoBlockStaticContent:
+    """
+    Represents static content associated with a video block (such as video files or related assets).
+    """
+    def __init__(  # pylint: disable=too-many-positional-arguments
+        self, loc, name, content_type, data, last_modified_at=None,
+        thumbnail_location=None, import_path=None, length=None, locked=False, content_digest=None
+    ):
         self.location = loc
-        self.name = name  # a display string which can be edited, and thus not part of the location which needs to be fixed  # lint-amnesty, pylint: disable=line-too-long
+        # a display string which can be edited, and thus not part of the location which needs to be fixed
+        self.name = name
         self.content_type = content_type
         self._data = data
         self.length = length
@@ -20,7 +31,7 @@ def __init__(self, loc, name, content_type, data, last_modified_at=None, thumbna
         self.content_digest = content_digest
 
     @staticmethod
-    def compute_location(course_key, path, revision=None, is_thumbnail=False):  # lint-amnesty, pylint: disable=unused-argument
+    def compute_location(course_key, path, revision=None, is_thumbnail=False):  # pylint: disable=unused-argument
         """
         Constructs a location object for static content.
 
@@ -48,6 +59,7 @@ def get_location_from_path(path):
             if path.startswith('/') or path.endswith('/'):
                 # try stripping off the leading slash and try again
                 return AssetKey.from_string(path.strip('/'))
+            return None
 
     @staticmethod
     def serialize_asset_key_with_slash(asset_key):

xblocks_contrib/video/mixin.py

@@ -5,9 +5,11 @@
 from xblock.core import XBlockMixin
 from xblock.fields import Scope, String
 
-# Make '_' a no-op so we can scrape strings. Using lambda instead of
+
+# Make '_' a no-op so we can scrape strings. Using dummy function instead of
 #  `django.utils.translation.gettext_noop` because Django cannot be imported in this file
-_ = lambda text: text
+def _(text):
+    return text
 
 
 class LicenseMixin(XBlockMixin):

xblocks_contrib/video/studio_metadata_mixin.py

@@ -123,7 +123,7 @@ def editable_metadata_fields(self):
         editable_fields.pop('sub')
 
         languages = [{'label': label, 'code': lang} for lang, label in settings.ALL_LANGUAGES]
-        languages.sort(key=lambda l: l['label'])
+        languages.sort(key=lambda lang_item: lang_item['label'])
         editable_fields['transcripts']['custom'] = True
         editable_fields['transcripts']['languages'] = languages
         editable_fields['transcripts']['type'] = 'VideoTranslations'
@@ -148,7 +148,9 @@ def editable_metadata_fields(self):
         if video_config_service:
             for sub_id in possible_sub_ids:
                 try:
-                    _, sub_id, _ = video_config_service.get_transcript(self, lang='en', output_format=TranscriptExtensions.TXT)
+                    _, sub_id, _ = video_config_service.get_transcript(
+                        self, lang='en', output_format=TranscriptExtensions.TXT
+                    )
                     transcripts_info['transcripts'] = dict(transcripts_info['transcripts'], en=sub_id)
                     break
                 except TranscriptNotFoundError:

xblocks_contrib/video/tests/__init__.py

@@ -0,0 +1,3 @@
+"""
+Tests for the video xblock.
+"""

xblocks_contrib/video/validation.py

@@ -15,7 +15,9 @@ class StudioValidationMessage(ValidationMessage):
 
     TYPES = [ValidationMessage.WARNING, ValidationMessage.ERROR, NOT_CONFIGURED]
 
-    def __init__(self, message_type, message_text, action_label=None, action_class=None, action_runtime_event=None):
+    def __init__(  # pylint: disable=too-many-positional-arguments
+        self, message_type, message_text, action_label=None, action_class=None, action_runtime_event=None
+    ):
         """
         Create a new message.
 

xblocks_contrib/video/video.py

@@ -1,12 +1,16 @@
-"""Video is ungraded Xmodule for support video content.
+"""
+Video is ungraded Xmodule for support video content.
 It's new improved video block, which support additional feature:
-- Can play non-YouTube video sources via in-browser HTML5 video player.
-- YouTube defaults to HTML5 mode from the start.
-- Speed changes in both YouTube and non-YouTube videos happen via
-in-browser HTML5 video method (when in HTML5 mode).
-- Navigational subtitles can be disabled altogether via an attribute
-in XML.
-Examples of html5 videos for manual testing:
+
+* Can play non-YouTube video sources via in-browser HTML5 video player.
+* YouTube defaults to HTML5 mode from the start.
+* Speed changes in both YouTube and non-YouTube videos happen via
+  in-browser HTML5 video method (when in HTML5 mode).
+* Navigational subtitles can be disabled altogether via an attribute
+  in XML.
+
+Examples of html5 videos for manual testing::
+
     https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.mp4
     https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.webm
     https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.ogv
@@ -92,23 +96,20 @@
 log = logging.getLogger(__name__)
 loader = ResourceLoader(__name__)
 
-# Make '_' a no-op so we can scrape strings. Using lambda instead of
-#  `django.utils.translation.ugettext_noop` because Django cannot be imported in this file
-_ = lambda text: text
-
 EXPORT_IMPORT_COURSE_DIR = 'course'
 EXPORT_IMPORT_STATIC_DIR = 'static'
 
 
 @XBlock.wants('settings', 'completion', 'request_cache', 'video_config')
 @XBlock.needs('i18n', 'user')
 class VideoBlock(
-    VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers,
-    StudioEditableXBlockMixin, LegacyXmlMixin, XBlock,
-    AjaxHandlerMixin, StudioMetadataMixin,
-    LicenseMixin):
+        VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers,
+        StudioEditableXBlockMixin, LegacyXmlMixin, XBlock,
+        AjaxHandlerMixin, StudioMetadataMixin,
+        LicenseMixin):
     """
-    XML source example:
+    XML source example::
+
         <video show_captions="true"
             youtube="0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg"
             url_name="lecture_21_3" display_name="S19V3: Vacancies"
@@ -315,7 +316,7 @@ def public_view(self, context):
         fragment.initialize_js("Video")
         return fragment
 
-    def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: disable=arguments-differ, too-many-statements
+    def get_html(self, view=STUDENT_VIEW, context=None):  # pylint: disable=too-many-statements
         """
         Return html for a given view of this block.
         """
@@ -460,7 +461,7 @@ def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: di
             'completionEnabled': completion_enabled,
             'completionPercentage': settings.COMPLETION_VIDEO_COMPLETE_PERCENTAGE,
             'duration': video_duration,
-            'end': self.end_time.total_seconds(),  # pylint: disable=no-member
+            'end': self.end_time.total_seconds(),
             'generalSpeed': self.global_speed,
             'lmsRootURL': settings.LMS_ROOT_URL,
             'poster': poster,
@@ -470,15 +471,15 @@ def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: di
             # this user, based on what was recorded the last time we saw the
             # user, and defaulting to True.
             'recordedYoutubeIsAvailable': self.youtube_is_available,
-            'savedVideoPosition': self.saved_video_position.total_seconds(),  # pylint: disable=no-member
+            'savedVideoPosition': self.saved_video_position.total_seconds(),
             'saveStateEnabled': not is_public_view,
             'saveStateUrl': self.ajax_url + '/save_user_state',
             # Despite the setting on the block, don't show transcript by default
             # if the video is embedded in social media
             'showCaptions': json.dumps(self.show_captions and not is_embed),
             'sources': sources,
             'speed': self.speed,
-            'start': self.start_time.total_seconds(),  # pylint: disable=no-member
+            'start': self.start_time.total_seconds(),
             'streams': self.youtube_streams,
             'transcriptAvailableTranslationsUrl': self.runtime.handler_url(
                 self, 'transcript', 'available_translations'
@@ -525,7 +526,8 @@ def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: di
             'poster': json.dumps(get_poster(self)),
             'track': track_url,
             'transcript_download_format': transcript_download_format,
-            'transcript_download_formats_list': self.fields['transcript_download_format'].values,  # lint-amnesty, pylint: disable=unsubscriptable-object
+            # pylint: disable=unsubscriptable-object
+            'transcript_download_formats_list': self.fields['transcript_download_format'].values,
             'transcript_feedback_enabled': self.is_transcript_feedback_enabled(),
         }
         video_config_service = self.runtime.service(self, 'video_config')
@@ -626,9 +628,10 @@ def parse_xml_new_runtime(cls, node, runtime, keys):
         video_block = runtime.construct_xblock_from_class(cls, keys)
         field_data = cls.parse_video_xml(node)
         for key, val in field_data.items():
-            if key not in cls.fields:  # lint-amnesty, pylint: disable=unsupported-membership-test
+            # pylint: disable=unsubscriptable-object
+            if key not in cls.fields:  # pylint: disable=unsupported-membership-test
                 continue  # parse_video_xml returns some old non-fields like 'source'
-            setattr(video_block, key, cls.fields[key].from_json(val))  # lint-amnesty, pylint: disable=unsubscriptable-object
+            setattr(video_block, key, cls.fields[key].from_json(val))
         # Don't use VAL in the new runtime:
         video_block.edx_video_id = None
         return video_block
@@ -663,6 +666,7 @@ def parse_xml(cls, node, runtime, _keys):
                 # value["filename"] = definition.get("filename", ["", None])
                 video.xml_attributes.update(value)
             elif field_name in video.fields:
+                # pylint: disable=unsubscriptable-object
                 setattr(video, field_name, cls.fields[field_name].from_json(value))
 
         # Update VAL with info extracted from `node`
@@ -699,7 +703,8 @@ def definition_to_xml(self, resource_fs):  # lint-amnesty, pylint: disable=too-m
             # Mild workaround to ensure that tests pass -- if a field
             # is set to its default value, we don't write it out.
             if value:
-                if key in self.fields and self.fields[key].is_set_on(self):  # lint-amnesty, pylint: disable=unsubscriptable-object, unsupported-membership-test
+                # pylint: disable=unsupported-membership-test,unsubscriptable-object
+                if key in self.fields and self.fields[key].is_set_on(self):
                     try:
                         xml.set(key, str(value))
                     except UnicodeDecodeError:
@@ -964,7 +969,8 @@ def parse_video_xml(cls, xml, id_generator=None):
             else:
                 # We export values with json.dumps (well, except for Strings, but
                 # for about a month we did it for Strings also).
-                field_data[attr] = deserialize_field(cls.fields[attr], value)  # lint-amnesty, pylint: disable=unsubscriptable-object
+                # pylint: disable-next=unsubscriptable-object
+                field_data[attr] = deserialize_field(cls.fields[attr], value)
 
         course_id = getattr(id_generator, 'target_course_id', None)
         # Update the handout location with current course_id
@@ -1080,7 +1086,8 @@ def request_cache(self):
     @request_cached(
         request_cache_getter=lambda args, kwargs: args[1],
     )
-    def get_cached_val_data_for_course(cls, request_cache, video_profile_names, course_id):  # lint-amnesty, pylint: disable=unused-argument
+    # pylint: disable=unused-argument
+    def get_cached_val_data_for_course(cls, request_cache, video_profile_names, course_id):
         """
         Returns the VAL data for the requested video profiles for the given course.
         """
@@ -1199,7 +1206,8 @@ def bind_for_student(self, user_id, wrappers=None):
         # Skip rebinding if we're already bound a user, and it's this user.
         if self.scope_ids.user_id is not None and user_id == self.scope_ids.user_id:
             if getattr(self.runtime, "position", None):
-                self.position = self.runtime.position  # update the position of the tab
+                # update the position of the tab
+                self.position = self.runtime.position  # pylint: disable=attribute-defined-outside-init
             return
 
         # If we are switching users mid-request, save the data from the old user.
@@ -1230,14 +1238,14 @@ def bind_for_student(self, user_id, wrappers=None):
             wrapped_field_data = self.runtime.service(self, "field-data-unbound")
             for wrapper in wrappers:
                 wrapped_field_data = wrapper(wrapped_field_data)
-            self._bound_field_data = wrapped_field_data
+            self._bound_field_data = wrapped_field_data  # pylint: disable=attribute-defined-outside-init
             if getattr(self.runtime, "uses_deprecated_field_data", False):
                 # This approach is deprecated but OldModuleStoreRuntime still requires it.
                 # For SplitModuleStoreRuntime, don't set ._field_data this way.
                 self._field_data = wrapped_field_data
 
     @classmethod
-    def definition_from_xml(cls, xml_object, system):  # lint-amnesty, pylint: disable=unused-argument
+    def definition_from_xml(cls, xml_object, system):
         if len(xml_object) == 0 and len(list(xml_object.items())) == 0:
             return {"data": ""}, []
         return {"data": etree.tostring(xml_object, pretty_print=True, encoding="unicode")}, []

xblocks_contrib/video/video_handlers.py

@@ -226,23 +226,26 @@ def transcript(self, request, dispatch):
         """
         Entry point for transcript handlers for student_view.
 
-        Request GET contains:
+        Request GET contains::
+
             (optional) `videoId` for `translation` dispatch.
             `is_bumper=1` flag for bumper case.
 
-        Dispatches, (HTTP GET):
+        Dispatches, (HTTP GET)::
+
             /translation/[language_id]
             /download
             /available_translations/
 
-        Explanations:
+        Explanations::
+
             `download`: returns SRT or TXT file.
             `translation`: depends on HTTP methods:
                     Provide translation for requested language, SJSON format is sent back on success,
                     Proper language_id should be in url.
             `available_translations`:
                     Returns list of languages, for which transcript files exist.
-                    For 'en' check if SJSON exists. For non-`en` check if SRT file exists.
+                    For 'en' check if SJSON exists. For non-'en' check if SRT file exists.
         """
         is_bumper = request.GET.get('is_bumper', False)
         transcripts = self.get_transcripts_info(is_bumper)
@@ -410,17 +413,20 @@ def studio_transcript(self, request, dispatch):
         """
         Entry point for Studio transcript handlers.
 
-        Dispatches:
-            /translation/[language_id] - language_id sould be in url.
+        Dispatches::
+
+            /translation/[language_id] - language_id should be in url.
+
+        ``translation`` dispatch support following HTTP methods::
 
-        `translation` dispatch support following HTTP methods:
             `POST`:
                 Upload srt file. Check possibility of generation of proper sjson files.
                 For now, it works only for self.transcripts, not for `en`.
-            `GET:
+            `GET`:
                 Return filename from storage. SRT format is sent back on success. Filename should be in GET dict.
 
-        We raise all exceptions right in Studio:
+        We raise all exceptions right in Studio::
+
             TranscriptNotFoundError:
                 Video or asset was deleted from module/contentstore, but request came later.
                 Seems impossible to be raised. block_render.py catches NotFoundErrors from here.