INCR-413: Run python-modernize and isort (#20786)

* INCR-413: Run python-modernize and isort

* INCR-413: Fix for text_type usage

common/lib/xmodule/xmodule/modulestore/draft_and_published.py

@@ -2,12 +2,17 @@
 This module provides an abstraction for Module Stores that support Draft and Published branches.
 """
 
-import threading
+from __future__ import absolute_import
+
 import logging
+import threading
 from abc import ABCMeta, abstractmethod
 from contextlib import contextmanager
+
+import six
 from six import text_type
-from . import ModuleStoreEnum, BulkOperationsMixin
+
+from . import BulkOperationsMixin, ModuleStoreEnum
 from .exceptions import ItemNotFoundError
 
 # Things w/ these categories should never be marked as version=DRAFT
@@ -67,12 +72,11 @@ class BranchSettingMixin(object):
             return self.default_branch_setting_func()
 
 
-class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin):
+class ModuleStoreDraftAndPublished(six.with_metaclass(ABCMeta, BranchSettingMixin, BulkOperationsMixin)):
     """
     A mixin for a read-write database backend that supports two branches, Draft and Published, with
     options to prefer Draft and fallback to Published.
     """
-    __metaclass__ = ABCMeta
 
     @abstractmethod
     def delete_item(self, location, user_id, revision=None, **kwargs):
@@ -167,8 +171,8 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin):
             self.update_item(old_parent_item, user_id)  # pylint: disable=no-member
             log.info(
                 '%s removed from %s children',
-                unicode(source_item.location),
-                unicode(old_parent_item.location)
+                text_type(source_item.location),
+                text_type(old_parent_item.location)
             )
 
         # Add item to new parent at particular location.
@@ -180,8 +184,8 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin):
             self.update_item(new_parent_item, user_id)  # pylint: disable=no-member
             log.info(
                 '%s added to %s children',
-                unicode(source_item.location),
-                unicode(new_parent_item.location)
+                text_type(source_item.location),
+                text_type(new_parent_item.location)
             )
 
         # Update parent attribute of the item block
@@ -189,8 +193,8 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin):
         self.update_item(source_item, user_id)  # pylint: disable=no-member
         log.info(
             '%s parent updated to %s',
-            unicode(source_item.location),
-            unicode(new_parent_item.location)
+            text_type(source_item.location),
+            text_type(new_parent_item.location)
         )
         return source_item.location
 

common/lib/xmodule/xmodule/modulestore/edit_info.py

 """
 Access methods to get EditInfo for xblocks
 """
-from xblock.core import XBlockMixin
+from __future__ import absolute_import
+
 from abc import ABCMeta, abstractmethod
 
+import six
+from xblock.core import XBlockMixin
+
 
 class EditInfoMixin(XBlockMixin):
     """
@@ -52,11 +56,10 @@ class EditInfoMixin(XBlockMixin):
         return self.runtime.get_published_on(self)
 
 
-class EditInfoRuntimeMixin(object):
+class EditInfoRuntimeMixin(six.with_metaclass(ABCMeta, object)):
     """
     An abstract mixin class for the functions which the :class: `EditInfoMixin` methods call on the runtime
     """
-    __metaclass__ = ABCMeta
 
     @abstractmethod
     def get_edited_by(self, xblock):

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -5,21 +5,24 @@ In this way, courses can be served up via either SplitMongoModuleStore or MongoM
 
 """
 
-import six
+from __future__ import absolute_import
+
+import functools
+import itertools
 import logging
 from contextlib import contextmanager
-import itertools
-import functools
-from contracts import contract, new_contract
 
+import six
+from contracts import contract, new_contract
 from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey, AssetKey
+from opaque_keys.edx.keys import AssetKey, CourseKey
 from opaque_keys.edx.locator import LibraryLocator
+
 from xmodule.assetstore import AssetMetadata
 
-from . import ModuleStoreWriteBase, ModuleStoreEnum, XMODULE_FIELDS_WITH_USAGE_KEYS
-from .exceptions import ItemNotFoundError, DuplicateCourseError
+from . import XMODULE_FIELDS_WITH_USAGE_KEYS, ModuleStoreEnum, ModuleStoreWriteBase
 from .draft_and_published import ModuleStoreDraftAndPublished
+from .exceptions import DuplicateCourseError, ItemNotFoundError
 from .split_migrator import SplitMigrator
 
 new_contract('CourseKey', CourseKey)
@@ -83,7 +86,7 @@ def strip_key(func):
                 if isinstance(field_value, list):
                     field_value = [strip_key_func(fv) for fv in field_value]
                 elif isinstance(field_value, dict):
-                    for key, val in field_value.iteritems():
+                    for key, val in six.iteritems(field_value):
                         field_value[key] = strip_key_func(val)
                 else:
                     field_value = strip_key_func(field_value)
@@ -123,7 +126,7 @@ def prepare_asides_to_store(asides_source):
         asides = []
         for asd in asides_source:
             aside_fields = {}
-            for asd_field_key, asd_field_val in asd.fields.iteritems():
+            for asd_field_key, asd_field_val in six.iteritems(asd.fields):
                 aside_fields[asd_field_key] = asd_field_val.read_from(asd)
             asides.append({
                 'aside_type': asd.scope_ids.block_type,
@@ -160,7 +163,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         self.modulestores = []
         self.mappings = {}
 
-        for course_id, store_name in mappings.iteritems():
+        for course_id, store_name in six.iteritems(mappings):
             try:
                 self.mappings[CourseKey.from_string(course_id)] = store_name
             except InvalidKeyError:
@@ -180,7 +183,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                 signal_handler=signal_handler,
             )
             # replace all named pointers to the store into actual pointers
-            for course_key, store_name in self.mappings.iteritems():
+            for course_key, store_name in six.iteritems(self.mappings):
                 if store_name == key:
                     self.mappings[course_key] = store
             self.modulestores.append(store)
@@ -308,7 +311,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                     )
                 else:
                     course_summaries[course_id] = course_summary
-        return course_summaries.values()
+        return list(course_summaries.values())
 
     @strip_key
     def get_courses(self, **kwargs):
@@ -323,7 +326,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                 if course_id not in courses:
                     # course is indeed unique. save it in result
                     courses[course_id] = course
-        return courses.values()
+        return list(courses.values())
 
     @strip_key
     def get_library_summaries(self, **kwargs):
@@ -340,7 +343,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                 library_id = self._clean_locator_for_mapping(library_summary.location)
                 if library_id not in library_summaries:
                     library_summaries[library_id] = library_summary
-        return library_summaries.values()
+        return list(library_summaries.values())
 
     @strip_key
     def get_libraries(self, **kwargs):
@@ -357,7 +360,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
                 if library_id not in libraries:
                     # library is indeed unique. save it in result
                     libraries[library_id] = library
-        return libraries.values()
+        return list(libraries.values())
 
     def make_course_key(self, org, course, run):
         """
@@ -367,7 +370,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         This key may represent a course that doesn't exist in this modulestore.
         """
         # If there is a mapping that match this org/course/run, use that
-        for course_id, store in self.mappings.iteritems():
+        for course_id, store in six.iteritems(self.mappings):
             candidate_key = store.make_course_key(org, course, run)
             if candidate_key == course_id:
                 return candidate_key
@@ -884,7 +887,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         # could be done in parallel threads if needed
         return dict(
             itertools.chain.from_iterable(
-                store.heartbeat().iteritems()
+                six.iteritems(store.heartbeat())
                 for store in self.modulestores
             )
         )

common/lib/xmodule/xmodule/modulestore/modulestore_settings.py

@@ -2,8 +2,12 @@
 This file contains helper functions for configuring module_store_setting settings and support for backward compatibility with older formats.
 """
 
-import warnings
+from __future__ import absolute_import
+
 import copy
+import warnings
+
+import six
 
 
 def convert_module_store_setting_if_needed(module_store_setting):
@@ -16,7 +20,7 @@ def convert_module_store_setting_if_needed(module_store_setting):
         Converts and returns the given stores in old (unordered) dict-style format to the new (ordered) list format
         """
         new_store_list = []
-        for store_name, store_settings in old_stores.iteritems():
+        for store_name, store_settings in six.iteritems(old_stores):
 
             store_settings['NAME'] = store_name
             if store_name == 'default':

common/lib/xmodule/xmodule/modulestore/split_migrator.py

@@ -6,11 +6,16 @@ Exists at the top level of modulestore b/c it needs to know about and access eac
 In general, it's strategy is to treat the other modulestores as read-only and to never directly
 manipulate storage but use existing api's.
 '''
+from __future__ import absolute_import
+
 import logging
 
+import six
+from opaque_keys.edx.locator import CourseLocator
+from six.moves import range
 from xblock.fields import Reference, ReferenceList, ReferenceValueDict
+
 from xmodule.modulestore import ModuleStoreEnum
-from opaque_keys.edx.locator import CourseLocator
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
 log = logging.getLogger(__name__)
@@ -48,7 +53,7 @@ class SplitMigrator(object):
         # create the course: set fields to explicitly_set for each scope, id_root = new_course_locator, master_branch = 'production'
         original_course = self.source_modulestore.get_course(source_course_key, **kwargs)
         if original_course is None:
-            raise ItemNotFoundError(unicode(source_course_key))
+            raise ItemNotFoundError(six.text_type(source_course_key))
 
         if new_org is None:
             new_org = source_course_key.org
@@ -139,12 +144,12 @@ class SplitMigrator(object):
                 # was in 'direct' so draft is a new version
                 split_module = self.split_modulestore.get_item(new_locator, **kwargs)
                 # need to remove any no-longer-explicitly-set values and add/update any now set values.
-                for name, field in split_module.fields.iteritems():
+                for name, field in six.iteritems(split_module.fields):
                     if field.is_set_on(split_module) and not module.fields[name].is_set_on(module):
                         field.delete_from(split_module)
-                for field, value in self._get_fields_translate_references(
+                for field, value in six.iteritems(self._get_fields_translate_references(
                         module, new_draft_course_loc, published_course_usage_key.block_id, field_names=False
-                ).iteritems():
+                )):
                     field.write_to(split_module, value)
 
                 _new_module = self.split_modulestore.update_item(split_module, user_id, **kwargs)
@@ -160,7 +165,7 @@ class SplitMigrator(object):
                     **kwargs
                 )
                 awaiting_adoption[module.location] = new_locator
-        for draft_location, new_locator in awaiting_adoption.iteritems():
+        for draft_location, new_locator in six.iteritems(awaiting_adoption):
             parent_loc = self.source_modulestore.get_parent_location(
                 draft_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred, **kwargs
             )
@@ -207,7 +212,7 @@ class SplitMigrator(object):
             )
 
         result = {}
-        for field_name, field in xblock.fields.iteritems():
+        for field_name, field in six.iteritems(xblock.fields):
             if field.is_set_on(xblock):
                 field_value = field.read_from(xblock)
                 field_key = field_name if field_names else field
@@ -220,7 +225,7 @@ class SplitMigrator(object):
                 elif isinstance(field, ReferenceValueDict):
                     result[field_key] = {
                         key: get_translation(subvalue)
-                        for key, subvalue in field_value.iteritems()
+                        for key, subvalue in six.iteritems(field_value)
                     }
                 else:
                     result[field_key] = field_value

common/lib/xmodule/xmodule/tests/xml/__init__.py

 """
 Xml parsing tests for XModules
 """
+from __future__ import absolute_import
+
 import pprint
+
 from django.test import TestCase
 from lxml import etree
 from mock import Mock
+from opaque_keys.edx.keys import CourseKey
 from six import text_type
+from xblock.runtime import DictKeyValueStore, KvsFieldData
 
-from xmodule.x_module import XMLParsingSystem, policy_key
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.modulestore.xml import CourseLocationManager
-from opaque_keys.edx.keys import CourseKey
-
-from xblock.runtime import KvsFieldData, DictKeyValueStore
+from xmodule.x_module import XMLParsingSystem, policy_key
 
 
 class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem):  # pylint: disable=abstract-method

common/lib/xmodule/xmodule/tests/xml/factories.py

@@ -2,17 +2,19 @@
 Factories for generating edXML for testing XModule import
 """
 
-import inspect
+from __future__ import absolute_import
 
+import inspect
 from tempfile import mkdtemp
+
+from factory import Factory, Sequence, lazy_attribute, post_generation
 from fs.osfs import OSFS
-from factory import Factory, lazy_attribute, post_generation, Sequence
 from lxml import etree
-
 from xblock.mixins import HierarchyMixin
+
+from xmodule.modulestore import only_xmodules
 from xmodule.modulestore.inheritance import InheritanceMixin
 from xmodule.x_module import XModuleMixin
-from xmodule.modulestore import only_xmodules
 
 
 class XmlImportData(object):

common/lib/xmodule/xmodule/tests/xml/test_inheritance.py

 """
 Test that inherited fields work correctly when parsing XML
 """
+from __future__ import absolute_import
+
 from xmodule.tests.xml import XModuleXmlImportTest
-from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory, XmlImportFactory
+from xmodule.tests.xml.factories import CourseFactory, ProblemFactory, SequenceFactory, XmlImportFactory
 
 
 class TestInheritedFieldParsing(XModuleXmlImportTest):

common/lib/xmodule/xmodule/tests/xml/test_policy.py

@@ -2,10 +2,12 @@
 Tests that policy json files import correctly when loading XML
 """
 
+from __future__ import absolute_import
+
 import pytest
 
-from xmodule.tests.xml.factories import CourseFactory
 from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml.factories import CourseFactory
 
 
 class TestPolicy(XModuleXmlImportTest):