Pylint and pep8 hygiene

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -28,8 +28,10 @@ class LocMapperStore(object):
 
     # C0103: varnames and attrs must be >= 3 chars, but db defined by long time usage
     # pylint: disable = C0103
-    def __init__(self, host, db, collection, port=27017, user=None, password=None,
-        **kwargs):
+    def __init__(
+        self, host, db, collection, port=27017, user=None, password=None,
+        **kwargs
+    ):
         '''
         Constructor
         '''

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

@@ -10,6 +10,7 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.locator import CourseLocator
 from xmodule.modulestore.mongo import draft
 
+
 class SplitMigrator(object):
     """
     Copies courses from old mongo to split mongo and sets up location mapping so any references to the old
@@ -58,7 +59,6 @@ class SplitMigrator(object):
 
         return new_course_id
 
-
     def _copy_published_modules_to_course(self, new_course, old_course_loc, old_course_id, user_id):
         """
         Copy all of the modules from the 'direct' version of the course to the new split course.
@@ -94,7 +94,6 @@ class SplitMigrator(object):
         # children which meant some pointers were to non-existent locations in 'direct'
         self.split_modulestore.internal_clean_children(course_version_locator)
 
-
     def _add_draft_modules_to_course(self, new_course_id, old_course_id, old_course_loc, user_id):
         """
         update each draft. Create any which don't exist in published and attach to their parents.
@@ -159,8 +158,10 @@ class SplitMigrator(object):
                 new_parent.children.insert(new_parent_cursor, new_usage_id)
                 new_parent = self.split_modulestore.update_item(new_parent, user_id)
 
-
     def _get_json_fields_translate_children(self, xblock, old_course_id, published):
+        """
+        Return the json repr for explicitly set fields but convert all children to their usage_id's
+        """
         fields = self.get_json_fields_explicitly_set(xblock)
         # this will too generously copy the children even for ones that don't exist in the published b/c the old mongo
         # had no way of not having parents point to draft only children :-(
@@ -172,7 +173,6 @@ class SplitMigrator(object):
                 for child in fields['children']]
         return fields
 
-
     def get_json_fields_explicitly_set(self, xblock):
         """
         Get the json repr for fields set on this specific xblock

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -642,9 +642,11 @@ class SplitMongoModuleStore(ModuleStoreBase):
     # DHM: Should I rewrite this to take a new xblock instance rather than to construct it? That is, require the
     # caller to use XModuleDescriptor.load_from_json thus reducing similar code and making the object creation and
     # validation behavior a responsibility of the model layer rather than the persistence layer.
-    def create_item(self, course_or_parent_locator, category, user_id,
+    def create_item(
+        self, course_or_parent_locator, category, user_id,
         usage_id=None, definition_locator=None, fields=None,
-        force=False, continue_version=False):
+        force=False, continue_version=False
+    ):
         """
         Add a descriptor to persistence as the last child of the optional parent_location or just as an element
         of the course (if no parent provided). Return the resulting post saved version with populated locators.
@@ -773,7 +775,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
         self, org, prettyid, user_id, id_root=None, fields=None,
         master_branch='draft', versions_dict=None, root_category='course',
         root_usage_id='course'
-        ):
+    ):
         """
         Create a new entry in the active courses index which points to an existing or new structure. Returns
         the course root of the resulting entry (the location has the course id)
@@ -848,9 +850,11 @@ class SplitMongoModuleStore(ModuleStoreBase):
             }
             new_id = self.structures.insert(draft_structure)
             draft_structure['original_version'] = new_id
-            self.structures.update({'_id': new_id},
+            self.structures.update(
+                {'_id': new_id},
                 {'$set': {"original_version": new_id,
-                    'blocks.{}.edit_info.update_version'.format(root_usage_id): new_id}})
+                    'blocks.{}.edit_info.update_version'.format(root_usage_id): new_id}}
+            )
             if versions_dict is None:
                 versions_dict = {master_branch: new_id}
             else:
@@ -1351,17 +1355,19 @@ class SplitMongoModuleStore(ModuleStoreBase):
                 return None
         else:
             index_entry = self.course_index.find_one({'_id': locator.course_id})
-            if (locator.version_guid is not None
-                and index_entry['versions'][locator.branch] != locator.version_guid
-                and not force) or (force and continue_version):
+            is_head = (
+                locator.version_guid is None or
+                index_entry['versions'][locator.branch] == locator.version_guid
+            )
+            if (is_head or (force and not continue_version)):
+                return index_entry
+            else:
                 raise VersionConflictError(
                     locator,
                     CourseLocator(
                         course_id=index_entry['_id'],
                         version_guid=index_entry['versions'][locator.branch],
                         branch=locator.branch))
-            else:
-                return index_entry
 
     def _version_structure(self, structure, user_id):
         """

common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py

@@ -23,6 +23,9 @@ from xmodule.modulestore.mongo import draft
 
 
 class TestMigration(unittest.TestCase):
+    """
+    Test the split migrator
+    """
 
     # Snippet of what would be in the django settings envs file
     db_config = {
@@ -113,18 +116,21 @@ class TestMigration(unittest.TestCase):
         self.create_random_units(self.old_mongo, both_vert, self.draft_mongo, draft_both)
         # vertical in draft only (x2)
         location = location.replace(category='vertical', name=uuid.uuid4().hex)
-        draft_vert = self._create_and_get_item(self.draft_mongo,
+        draft_vert = self._create_and_get_item(
+            self.draft_mongo,
             location, {}, {'display_name': 'Draft vertical'}, runtime)
         chapter1.children.append(draft_vert.location.url())
         self.create_random_units(self.draft_mongo, draft_vert)
         location = location.replace(category='vertical', name=uuid.uuid4().hex)
-        draft_vert = self._create_and_get_item(self.draft_mongo,
+        draft_vert = self._create_and_get_item(
+            self.draft_mongo,
             location, {}, {'display_name': 'Draft vertical2'}, runtime)
         chapter1.children.append(draft_vert.location.url())
         self.create_random_units(self.draft_mongo, draft_vert)
         # and finally one in live only (so published has to skip 2)
         location = location.replace(category='vertical', name=uuid.uuid4().hex)
-        live_vert = self._create_and_get_item(self.old_mongo,
+        live_vert = self._create_and_get_item(
+            self.old_mongo,
             location, {}, {'display_name': 'Live vertical end'}, runtime)
         chapter1.children.append(live_vert.location.url())
         self.create_random_units(self.old_mongo, live_vert)
@@ -134,17 +140,19 @@ class TestMigration(unittest.TestCase):
 
         # now the other one w/ the conditional
         # first create some show children
-        indirect1 = self._create_and_get_item(self.old_mongo,
+        indirect1 = self._create_and_get_item(
+            self.old_mongo,
             location.replace(category='discussion', name=uuid.uuid4().hex),
             "", {'display_name': 'conditional show 1'}, runtime
         )
-        indirect2 = self._create_and_get_item(self.old_mongo,
+        indirect2 = self._create_and_get_item(
+            self.old_mongo,
             location.replace(category='html', name=uuid.uuid4().hex),
             "", {'display_name': 'conditional show 2'}, runtime
         )
         location = location.replace(category='conditional', name=uuid.uuid4().hex)
         metadata = {
-            'xml_attributes' : {
+            'xml_attributes': {
                 'sources': [live_vert.location.url(), ],
                 'completed': True,
             },
@@ -167,11 +175,11 @@ class TestMigration(unittest.TestCase):
         location = location.replace(category='about', name='overview')
         _overview = self._create_and_get_item(self.old_mongo, location, "<p>test</p>", {}, runtime)
         location = location.replace(category='course_info', name='updates')
-        _overview = self._create_and_get_item(self.old_mongo,
+        _overview = self._create_and_get_item(
+            self.old_mongo,
             location, "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, runtime
         )
 
-
     def create_random_units(self, store, parent, cc_store=None, cc_parent=None):
         """
         Create a random selection of units under the given parent w/ random names & attrs
@@ -218,7 +226,6 @@ class TestMigration(unittest.TestCase):
                 )
                 self.compare_dags(presplit, conditional, self.split_mongo.get_item(locator), published)
 
-
     def compare_dags(self, presplit, presplit_dag_root, split_dag_root, published):
         # check that locations match
         self.assertEqual(