INCR-227: python-modernize on common/lib/xmodule/xmodule/video_module… (#20427)

* INCR-227: python-modernize on common/lib/xmodule/xmodule/video_module, partitions, and util

* pylint will ignore imports from six

* INCR-227 Fixing mangled comments and variable names

common/lib/xmodule/xmodule/partitions/partitions.py

@@ -1,5 +1,7 @@
 """Defines ``Group`` and ``UserPartition`` models for partitioning"""
 
+from __future__ import absolute_import
+
 from collections import namedtuple
 
 from stevedore.extension import ExtensionManager

common/lib/xmodule/xmodule/partitions/partitions_service.py

@@ -3,19 +3,18 @@ This is a service-like API that assigns tracks which groups users are in for var
 user partitions.  It uses the user_service key/value store provided by the LMS runtime to
 persist the assignments.
 """
+from __future__ import absolute_import
+
+import logging
+
+import six
 from django.conf import settings
 from django.utils.translation import ugettext_lazy as _
-import logging
 
 from openedx.core.lib.cache_utils import request_cached
 from openedx.features.content_type_gating.partitions import create_content_gating_partition
-from xmodule.partitions.partitions import (
-    UserPartition,
-    UserPartitionError,
-    ENROLLMENT_TRACK_PARTITION_ID,
-)
 from xmodule.modulestore.django import modulestore
-
+from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID, UserPartition, UserPartitionError
 
 log = logging.getLogger(__name__)
 
@@ -105,7 +104,7 @@ def _create_enrollment_track_partition(course):
             "Can't add 'enrollment_track' partition, as ID {id} is assigned to {partition} in course {course}.".format(
                 id=ENROLLMENT_TRACK_PARTITION_ID,
                 partition=get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name,
-                course=unicode(course.id)
+                course=six.text_type(course.id)
             )
         )
         return None
@@ -114,7 +113,7 @@ def _create_enrollment_track_partition(course):
         id=ENROLLMENT_TRACK_PARTITION_ID,
         name=_(u"Enrollment Track Groups"),
         description=_(u"Partition for segmenting users by enrollment track"),
-        parameters={"course_id": unicode(course.id)}
+        parameters={"course_id": six.text_type(course.id)}
     )
     return partition
 

common/lib/xmodule/xmodule/partitions/tests/test_partitions.py

@@ -3,20 +3,26 @@ Test the partitions and partitions service
 
 """
 
+from __future__ import absolute_import
+
 from datetime import datetime
+
+import six
 from django.test import TestCase
 from mock import Mock, patch
-
 from opaque_keys.edx.locator import CourseLocator
 from stevedore.extension import Extension, ExtensionManager
-from xmodule.partitions.partitions import (
-    Group, UserPartition, UserPartitionError, NoSuchUserPartitionGroupError,
-    USER_PARTITION_SCHEME_NAMESPACE, ENROLLMENT_TRACK_PARTITION_ID
-)
-from xmodule.partitions.partitions_service import (
-    PartitionService, get_all_partitions_for_course, FEATURES
-)
+
 from openedx.features.content_type_gating.models import ContentTypeGatingConfig
+from xmodule.partitions.partitions import (
+    ENROLLMENT_TRACK_PARTITION_ID,
+    USER_PARTITION_SCHEME_NAMESPACE,
+    Group,
+    NoSuchUserPartitionGroupError,
+    UserPartition,
+    UserPartitionError
+)
+from xmodule.partitions.partitions_service import FEATURES, PartitionService, get_all_partitions_for_course
 
 
 class TestGroup(TestCase):
@@ -574,7 +580,7 @@ class TestGetCourseUserPartitions(PartitionServiceBaseClass):
         self.assertEqual(self.TEST_SCHEME_NAME, all_partitions[0].scheme.name)
         enrollment_track_partition = all_partitions[1]
         self.assertEqual(self.ENROLLMENT_TRACK_SCHEME_NAME, enrollment_track_partition.scheme.name)
-        self.assertEqual(unicode(self.course.id), enrollment_track_partition.parameters['course_id'])
+        self.assertEqual(six.text_type(self.course.id), enrollment_track_partition.parameters['course_id'])
         self.assertEqual(ENROLLMENT_TRACK_PARTITION_ID, enrollment_track_partition.id)
 
     def test_enrollment_track_partition_not_added_if_conflict(self):

common/lib/xmodule/xmodule/util/duedate.py

@@ -1,6 +1,8 @@
 """
 Miscellaneous utility functions.
 """
+from __future__ import absolute_import
+
 from functools import partial
 
 

common/lib/xmodule/xmodule/util/misc.py

@@ -1,6 +1,8 @@
 """
 Miscellaneous utility functions.
 """
+from __future__ import absolute_import
+
 import re
 
 from xmodule.annotator_mixin import html_to_text

common/lib/xmodule/xmodule/util/sandboxing.py

@@ -1,4 +1,8 @@
+from __future__ import absolute_import
+
 import re
+
+import six
 from django.conf import settings
 
 DEFAULT_PYTHON_LIB_FILENAME = 'python_lib.zip'
@@ -24,7 +28,7 @@ def can_execute_unsafe_code(course_id):
     # To others using this: the code as-is is brittle and likely to be changed in the future,
     # as per the TODO, so please consider carefully before adding more values to COURSES_WITH_UNSAFE_CODE
     for regex in getattr(settings, 'COURSES_WITH_UNSAFE_CODE', []):
-        if re.match(regex, unicode(course_id)):
+        if re.match(regex, six.text_type(course_id)):
             return True
     return False
 

common/lib/xmodule/xmodule/util/xmodule_django.py

@@ -4,9 +4,11 @@ NOTE: This file should only be imported into 'django-safe' code, i.e. known that
 runtime environment with the djangoapps in common configured to load
 """
 
+from __future__ import absolute_import
+
+import webpack_loader
 # NOTE: we are importing this method so that any module that imports us has access to get_current_request
 from crum import get_current_request
-import webpack_loader
 
 
 def get_current_request_hostname():

common/lib/xmodule/xmodule/video_module/__init__.py

@@ -4,7 +4,7 @@ Container for video module and its utils.
 
 # pylint: disable=wildcard-import
 
-from .transcripts_utils import *
-from .video_utils import *
-from .video_module import *
 from .bumper_utils import *
+from .transcripts_utils import *
+from .video_module import *
+from .video_utils import *

common/lib/xmodule/xmodule/video_module/bumper_utils.py

@@ -1,14 +1,16 @@
 """
 Utils for video bumper
 """
-from collections import OrderedDict
+from __future__ import absolute_import
+
 import copy
 import json
 import logging
-
+from collections import OrderedDict
 from datetime import datetime, timedelta
-from django.conf import settings
+
 import pytz
+from django.conf import settings
 
 from .video_utils import set_query_parameter
 
@@ -106,7 +108,7 @@ def get_bumper_sources(video):
     try:
         val_profiles = ["desktop_webm", "desktop_mp4"]
         val_video_urls = edxval_api.get_urls_for_profiles(video.bumper['edx_video_id'], val_profiles)
-        bumper_sources = filter(None, [val_video_urls[p] for p in val_profiles])
+        bumper_sources = [url for url in [val_video_urls[p] for p in val_profiles] if url]
     except edxval_api.ValInternalError:
         # if no bumper sources, nothing will be showed
         log.warning(

common/lib/xmodule/xmodule/video_module/transcripts_utils.py

@@ -2,22 +2,27 @@
 Utility functions for transcripts.
 ++++++++++++++++++++++++++++++++++
 """
-from functools import wraps
-from django.conf import settings
-import os
+from __future__ import absolute_import
+
 import copy
 import json
-import requests
 import logging
-from pysrt import SubRipTime, SubRipItem, SubRipFile
-from pysrt.srtexc import Error
-from lxml import etree
-from HTMLParser import HTMLParser
-from six import text_type
+import os
+from functools import wraps
+
+import requests
+import six
+from django.conf import settings
+from lxml import etree
+from pysrt import SubRipFile, SubRipItem, SubRipTime
+from pysrt.srtexc import Error
+from six import text_type
+from six.moves import range, zip
+from six.moves.html_parser import HTMLParser  # pylint: disable=import-error
 
-from xmodule.exceptions import NotFoundError
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
+from xmodule.exceptions import NotFoundError
 
 from .bumper_utils import get_bumper_settings
 
@@ -266,7 +271,7 @@ def generate_subs_from_source(speed_subs, subs_type, subs_filedata, item, langua
         'end': sub_ends,
         'text': sub_texts}
 
-    for speed, subs_id in speed_subs.iteritems():
+    for speed, subs_id in six.iteritems(speed_subs):
         save_subs_to_store(
             generate_subs(speed, 1, subs),
             subs_id,
@@ -300,7 +305,7 @@ def generate_srt_from_sjson(sjson_subs, speed):
             end=SubRipTime(milliseconds=sjson_speed_1['end'][i]),
             text=sjson_speed_1['text'][i]
         )
-        output += (unicode(item))
+        output += (six.text_type(item))
         output += '\n'
     return output
 
@@ -439,7 +444,7 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat
                 generate_sjson_for_all_speeds(
                     item,
                     item.transcripts[lang],
-                    {speed: subs_id for subs_id, speed in youtube_speed_dict(item).iteritems()},
+                    {speed: subs_id for subs_id, speed in six.iteritems(youtube_speed_dict(item))},
                     lang,
                 )
             except TranscriptException as ex:
@@ -537,13 +542,13 @@ def get_video_ids_info(edx_video_id, youtube_id_1_0, html5_sources):
     Returns:
         tuple: external or internal, video ids list
     """
-    clean = lambda item: item.strip() if isinstance(item, basestring) else item
+    clean = lambda item: item.strip() if isinstance(item, six.string_types) else item
     external = not bool(clean(edx_video_id))
 
     video_ids = [edx_video_id, youtube_id_1_0] + get_html5_ids(html5_sources)
 
     # video_ids cleanup
-    video_ids = filter(lambda item: bool(clean(item)), video_ids)
+    video_ids = [item for item in video_ids if bool(clean(item))]
 
     return external, video_ids
 
@@ -755,7 +760,7 @@ class VideoTranscriptsMixin(object):
             if sub:
                 all_langs.update({'en': sub})
 
-            for language, filename in all_langs.iteritems():
+            for language, filename in six.iteritems(all_langs):
                 try:
                     # for bumper videos, transcripts are stored in content store only
                     if is_bumper:

common/lib/xmodule/xmodule/video_module/video_handlers.py

@@ -5,35 +5,36 @@ StudentViewHandlers are handlers for video module instance.
 StudioViewHandlers are handlers for video descriptor instance.
 """
 
+from __future__ import absolute_import
+
 import json
 import logging
 
 import six
 from django.core.files.base import ContentFile
 from django.utils.timezone import now
+from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript
+from opaque_keys.edx.locator import CourseLocator
 from webob import Response
-
 from xblock.core import XBlock
 from xblock.exceptions import JsonHandlerError
 
 from xmodule.exceptions import NotFoundError
 from xmodule.fields import RelativeTime
-from opaque_keys.edx.locator import CourseLocator
 
-from edxval.api import create_or_update_video_transcript, create_external_video, delete_video_transcript
 from .transcripts_utils import (
-    clean_video_id,
-    get_or_create_sjson,
-    generate_sjson_for_all_speeds,
-    subs_filename,
     Transcript,
     TranscriptException,
     TranscriptsGenerationException,
-    youtube_speed_dict,
+    clean_video_id,
+    generate_sjson_for_all_speeds,
+    get_html5_ids,
+    get_or_create_sjson,
     get_transcript,
     get_transcript_from_contentstore,
     remove_subs_from_store,
-    get_html5_ids
+    subs_filename,
+    youtube_speed_dict
 )
 
 log = logging.getLogger(__name__)
@@ -153,7 +154,7 @@ class VideoStudentViewHandlers(object):
                 generate_sjson_for_all_speeds(
                     self,
                     other_lang[self.transcript_language],
-                    {speed: youtube_id for youtube_id, speed in youtube_ids.iteritems()},
+                    {speed: youtube_id for youtube_id, speed in six.iteritems(youtube_ids)},
                     self.transcript_language
                 )
                 sjson_transcript = Transcript.asset(self.location, youtube_id, self.transcript_language).data
@@ -309,7 +310,7 @@ class VideoStudentViewHandlers(object):
                 log.info("Invalid /translation request: no language.")
                 return Response(status=400)
 
-            if language not in ['en'] + transcripts["transcripts"].keys():
+            if language not in ['en'] + list(transcripts["transcripts"].keys()):
                 log.info("Video: transcript facilities are not available for given language.")
                 return Response(status=404)
 

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -12,25 +12,29 @@ Examples of html5 videos for manual testing:
     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
 """
+from __future__ import absolute_import
+
 import copy
 import json
 import logging
-from collections import defaultdict, OrderedDict
+from collections import OrderedDict, defaultdict
 from operator import itemgetter
 
-from pkg_resources import resource_string
-
+import six
 from django.conf import settings
 from lxml import etree
 from opaque_keys.edx.locator import AssetLocator
-from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag
-from openedx.core.djangoapps.video_pipeline.config.waffle import waffle_flags, DEPRECATE_YOUTUBE
-from openedx.core.lib.cache_utils import request_cached
-from openedx.core.lib.license import LicenseMixin
+from pkg_resources import resource_string
+from web_fragments.fragment import Fragment
 from xblock.completable import XBlockCompletionMode
 from xblock.core import XBlock
 from xblock.fields import ScopeIds
 from xblock.runtime import KvsFieldData
+
+from openedx.core.djangoapps.video_config.models import HLSPlaybackEnabledFlag
+from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE, waffle_flags
+from openedx.core.lib.cache_utils import request_cached
+from openedx.core.lib.license import LicenseMixin
 from xmodule.contentstore.content import StaticContent
 from xmodule.editing_module import TabsEditingDescriptor
 from xmodule.exceptions import NotFoundError
@@ -38,23 +42,21 @@ from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metada
 from xmodule.raw_module import EmptyDataRawDescriptor
 from xmodule.validation import StudioValidation, StudioValidationMessage
 from xmodule.video_module import manage_video_subtitles_save
-from xmodule.x_module import XModule, module_attr, PUBLIC_VIEW, STUDENT_VIEW
+from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW, XModule, module_attr
 from xmodule.xml_module import deserialize_field, is_pointer_tag, name_to_pathname
 
 from .bumper_utils import bumperize
 from .transcripts_utils import (
-    get_html5_ids,
     Transcript,
     VideoTranscriptsMixin,
     clean_video_id,
-    subs_filename,
-    get_transcript_for_video
+    get_html5_ids,
+    get_transcript_for_video,
+    subs_filename
 )
-
 from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers
 from .video_utils import create_youtube_string, format_xml_exception_message, get_poster, rewrite_video_url
 from .video_xfields import VideoFields
-from web_fragments.fragment import Fragment
 
 # The following import/except block for edxval is temporary measure until
 # edxval is a proper XBlock Runtime Service.
@@ -176,7 +178,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
             languages['en'] = 'English'
 
         # OrderedDict for easy testing of rendered context in tests
-        sorted_languages = sorted(languages.items(), key=itemgetter(1))
+        sorted_languages = sorted(list(languages.items()), key=itemgetter(1))
 
         sorted_languages = OrderedDict(sorted_languages)
         return track_url, transcript_language, sorted_languages
@@ -219,7 +221,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
 
         track_status = (self.download_track and self.track)
         transcript_download_format = self.transcript_download_format if not track_status else None
-        sources = filter(None, self.html5_sources)
+        sources = [source for source in self.html5_sources if source]
 
         download_video_link = None
         branding_info = None
@@ -687,7 +689,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
         # Mild workaround to ensure that tests pass -- if a field
         # is set to its default value, we don't need to write it out.
         if youtube_string and youtube_string != '1.00:3_yD_cEKoCk':
-            xml.set('youtube', unicode(youtube_string))
+            xml.set('youtube', six.text_type(youtube_string))
         xml.set('url_name', self.url_name)
         attrs = {
             'display_name': self.display_name,
@@ -704,13 +706,13 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             if value:
                 if key in self.fields and self.fields[key].is_set_on(self):
                     try:
-                        xml.set(key, unicode(value))
+                        xml.set(key, six.text_type(value))
                     except UnicodeDecodeError:
                         exception_message = format_xml_exception_message(self.location, key, value)
                         log.exception(exception_message)
                         # If exception is UnicodeDecodeError set value using unicode 'utf-8' scheme.
                         log.info("Setting xml value using 'utf-8' scheme.")
-                        xml.set(key, unicode(value, 'utf-8'))
+                        xml.set(key, six.text_type(value, 'utf-8'))
                     except ValueError:
                         exception_message = format_xml_exception_message(self.location, key, value)
                         log.exception(exception_message)
@@ -752,7 +754,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                     video_id=edx_video_id,
                     resource_fs=resource_fs,
                     static_dir=EXPORT_IMPORT_STATIC_DIR,
-                    course_id=unicode(self.runtime.course_id.for_branch(None))
+                    course_id=six.text_type(self.runtime.course_id.for_branch(None))
                 )
                 # Update xml with edxval metadata
                 xml.append(exported_metadata['xml'])
@@ -957,7 +959,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
 
         course_id = getattr(id_generator, 'target_course_id', None)
         # Update the handout location with current course_id
-        if 'handout' in field_data.keys() and course_id:
+        if 'handout' in list(field_data.keys()) and course_id:
             handout_location = StaticContent.get_location_from_path(field_data['handout'])
             if isinstance(handout_location, AssetLocator):
                 handout_new_location = StaticContent.compute_location(course_id, handout_location.path)
@@ -1072,7 +1074,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
         """
         Returns the VAL data for the requested video profiles for the given course.
         """
-        return edxval_api.get_video_info_for_course_and_profiles(unicode(course_id), video_profile_names)
+        return edxval_api.get_video_info_for_course_and_profiles(six.text_type(course_id), video_profile_names)
 
     def student_view_data(self, context=None):
         """

common/lib/xmodule/xmodule/video_module/video_utils.py

@@ -3,14 +3,17 @@
 Module contains utils specific for video_module but not for transcripts.
 """
 
-from collections import OrderedDict
-import logging
-from urllib import urlencode
-from urlparse import parse_qs, urlsplit, urlunsplit, urlparse
+from __future__ import absolute_import
 
+import logging
+from collections import OrderedDict
+
+import six
 from django.conf import settings
-from django.core.validators import URLValidator
 from django.core.exceptions import ValidationError
+from django.core.validators import URLValidator
+from six.moves import zip
+from six.moves.urllib.parse import parse_qs, urlencode, urlparse, urlsplit, urlunsplit  # pylint: disable=import-error
 
 log = logging.getLogger(__name__)
 
@@ -104,7 +107,7 @@ def format_xml_exception_message(location, key, value):
     when setting xml attributes.
     """
     exception_message = "Block-location:{location}, Key:{key}, Value:{value}".format(
-        location=unicode(location),
+        location=six.text_type(location),
         key=key,
         value=value
     )

common/lib/xmodule/xmodule/video_module/video_xfields.py

@@ -1,9 +1,12 @@
 """
 XFields for video module.
 """
+from __future__ import absolute_import
+
 import datetime
 
-from xblock.fields import Scope, String, Float, Boolean, List, Dict, DateTime
+from xblock.fields import Boolean, DateTime, Dict, Float, List, Scope, String
+
 from xmodule.fields import RelativeTime
 
 # Make '_' a no-op so we can scrape strings. Using lambda instead of