Merge pull request #268 from edx/dhm/cosmetic-cleanup

Cosmetic Cleanups Incidental to Next Gen Modulestore Implementaton

cms/djangoapps/contentstore/features/video-editor.py

 # disable missing docstring
-#pylint: disable=C0111
+# pylint: disable=C0111
 
 from lettuce import world, step
 

cms/djangoapps/contentstore/module_info_model.py

@@ -5,10 +5,7 @@ from xmodule.modulestore import Location
 
 def get_module_info(store, location, parent_location=None, rewrite_static_links=False):
     try:
-        if location.revision is None:
-            module = store.get_item(location)
-        else:
-            module = store.get_item(location)
+        module = store.get_item(location)
     except ItemNotFoundError:
         # create a new one
         template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty'])

cms/djangoapps/contentstore/utils.py

@@ -10,7 +10,7 @@ from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
 
 log = logging.getLogger(__name__)
 
-#In order to instantiate an open ended tab automatically, need to have this data
+# In order to instantiate an open ended tab automatically, need to have this data
 OPEN_ENDED_PANEL = {"name": "Open Ended Panel", "type": "open_ended"}
 NOTES_PANEL = {"name": "My Notes", "type": "notes"}
 EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]])

cms/djangoapps/contentstore/views/assets.py

@@ -240,13 +240,13 @@ def import_course(request, org, course, name):
         # find the 'course.xml' file
 
         for dirpath, _dirnames, filenames in os.walk(course_dir):
-            for files in filenames:
-                if files == 'course.xml':
+            for filename in filenames:
+                if filename == 'course.xml':
                     break
-            if files == 'course.xml':
+            if filename == 'course.xml':
                 break
 
-        if files != 'course.xml':
+        if filename != 'course.xml':
             return HttpResponse(json.dumps({'ErrMsg': 'Could not find the course.xml file in the package.'}))
 
         logging.debug('found course.xml at {0}'.format(dirpath))

cms/djangoapps/contentstore/views/session_kv_store.py

@@ -2,27 +2,27 @@ from xblock.runtime import KeyValueStore, InvalidScopeError
 
 
 class SessionKeyValueStore(KeyValueStore):
-    def __init__(self, request, model_data):
-        self._model_data = model_data
+    def __init__(self, request, descriptor_model_data):
+        self._descriptor_model_data = descriptor_model_data
         self._session = request.session
 
     def get(self, key):
         try:
-            return self._model_data[key.field_name]
+            return self._descriptor_model_data[key.field_name]
         except (KeyError, InvalidScopeError):
             return self._session[tuple(key)]
 
     def set(self, key, value):
         try:
-            self._model_data[key.field_name] = value
+            self._descriptor_model_data[key.field_name] = value
         except (KeyError, InvalidScopeError):
             self._session[tuple(key)] = value
 
     def delete(self, key):
         try:
-            del self._model_data[key.field_name]
+            del self._descriptor_model_data[key.field_name]
         except (KeyError, InvalidScopeError):
             del self._session[tuple(key)]
 
     def has(self, key):
-        return key in self._model_data or key in self._session
+        return key in self._descriptor_model_data or key in self._session

cms/envs/acceptance.py

@@ -23,7 +23,7 @@ MODULESTORE_OPTIONS = {
     'db': 'test_xmodule',
     'collection': 'acceptance_modulestore',
     'fs_root': TEST_ROOT / "data",
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

cms/envs/dev.py

@@ -22,7 +22,7 @@ modulestore_options = {
     'db': 'xmodule',
     'collection': 'modulestore',
     'fs_root': GITHUB_REPO_ROOT,
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

cms/envs/test.py

@@ -48,7 +48,7 @@ MODULESTORE_OPTIONS = {
     'db': 'test_xmodule',
     'collection': 'test_modulestore',
     'fs_root': TEST_ROOT / "data",
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

cms/templates/new_item.html

-<section>
-  <div>${parent_name}</div>
-  <div>${parent_location}</div>
-  <input type="text" class="name"/>
-  <div>
-    % for module_type, module_templates in templates:
-    <div>
-      <div>${module_type}</div>
-      <div>
-        % for template in module_templates:
-        <a class="save" data-template-id="${template.location.url()}">${template.display_name_with_default}</a>
-        % endfor
-      </div>
-    </div>
-    % endfor
-  </div>
-  <a class='cancel'>Cancel</a>
-</section>
-

cms/templates/overview.html

@@ -167,7 +167,8 @@
                 %else:
                   <span class="published-status"><strong>Will Release:</strong> 
                   ${date_utils.get_default_time_display(section.lms.start)}</span>
-                  <a href="#" class="edit-button" data-date="${start_date_str}" data-time="${start_time_str}" data-id="${section.location}">Edit</a>
+                  <a href="#" class="edit-button" data-date="${start_date_str}" 
+                    data-time="${start_time_str}" data-id="${section.location}">Edit</a>
                 %endif
               </div>
             </div>

common/lib/xmodule/xmodule/course_module.py

@@ -212,6 +212,9 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
     template_dir_name = 'course'
 
     def __init__(self, *args, **kwargs):
+        """
+        Expects the same arguments as XModuleDescriptor.__init__
+        """
         super(CourseDescriptor, self).__init__(*args, **kwargs)
 
         if self.wiki_slug is None:

common/lib/xmodule/xmodule/modulestore/draft.py

@@ -101,12 +101,12 @@ class DraftModuleStore(ModuleStoreBase):
         draft_items = super(DraftModuleStore, self).get_items(draft_loc, course_id=course_id, depth=depth)
         items = super(DraftModuleStore, self).get_items(location, course_id=course_id, depth=depth)
 
-        draft_locs_found = set(item.location._replace(revision=None) for item in draft_items)
+        draft_locs_found = set(item.location.replace(revision=None) for item in draft_items)
         non_draft_items = [
             item
             for item in items
             if (item.location.revision != DRAFT
-                and item.location._replace(revision=None) not in draft_locs_found)
+                and item.location.replace(revision=None) not in draft_locs_found)
         ]
         return [wrap_draft(item) for item in draft_items + non_draft_items]
 

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -195,7 +195,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
                 if self.cached_metadata is not None:
                     # parent container pointers don't differentiate between draft and non-draft
                     # so when we do the lookup, we should do so with a non-draft location
-                    non_draft_loc = location._replace(revision=None)
+                    non_draft_loc = location.replace(revision=None)
                     metadata_to_inherit = self.cached_metadata.get(non_draft_loc.url(), {})
                     inherit_metadata(module, metadata_to_inherit)
                 return module

common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py

@@ -646,7 +646,7 @@ class CombinedOpenEndedV1Module():
         if self.student_attempts > self.attempts:
             return {
                 'success': False,
-                #This is a student_facing_error
+                # This is a student_facing_error
                 'error': (
                     'You have attempted this question {0} times.  '
                     'You are only allowed to attempt it {1} times.'

common/lib/xmodule/xmodule/tests/test_import.py

@@ -157,9 +157,10 @@ class ImportTestCase(BaseCourseTestCase):
         self.assertEqual(child.lms.due, ImportTestCase.date.from_json(v))
         self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
         self.assertEqual(2, len(child._inherited_metadata))
-        self.assertLessEqual(ImportTestCase.date.from_json(
-            child._inherited_metadata['start']),
-            datetime.datetime.now(UTC()))
+        self.assertLessEqual(
+            ImportTestCase.date.from_json(child._inherited_metadata['start']),
+            datetime.datetime.now(UTC())
+        )
         self.assertEqual(v, child._inherited_metadata['due'])
 
         # Now export and check things
@@ -221,7 +222,8 @@ class ImportTestCase(BaseCourseTestCase):
         # why do these tests look in the internal structure v just calling child.start?
         self.assertLessEqual(
             ImportTestCase.date.from_json(child._inherited_metadata['start']),
-            datetime.datetime.now(UTC()))
+            datetime.datetime.now(UTC())
+        )
 
     def test_metadata_override_default(self):
         """

common/lib/xmodule/xmodule/tests/test_xml_module.py

@@ -248,7 +248,7 @@ class TestDeserializeFloat(TestDeserialize):
     test_field = Float
 
     def test_deserialize(self):
-        self.assertDeserializeEqual( -2, '-2')
+        self.assertDeserializeEqual(-2, '-2')
         self.assertDeserializeEqual("450", '"450"')
         self.assertDeserializeEqual(-2.78, '-2.78')
         self.assertDeserializeEqual("0.45", '"0.45"')
@@ -256,7 +256,7 @@ class TestDeserializeFloat(TestDeserialize):
         # False can be parsed as a float (converts to 0)
         self.assertDeserializeEqual(False, 'false')
         # True can be parsed as a float (converts to 1)
-        self.assertDeserializeEqual( True, 'true')
+        self.assertDeserializeEqual(True, 'true')
 
     def test_deserialize_unsupported_types(self):
         self.assertDeserializeEqual('[3]', '[3]')

common/lib/xmodule/xmodule/xml_module.py

@@ -141,9 +141,9 @@ class XmlDescriptor(XModuleDescriptor):
     # Related: What's the right behavior for clean_metadata?
     metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize',
         'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc',
-        'ispublic', 	# if True, then course is listed for all users; see
-        'xqa_key',  	# for xqaa server access
-        'giturl',	# url of git server for origin of file
+        'ispublic',  # if True, then course is listed for all users; see
+        'xqa_key',  # for xqaa server access
+        'giturl',  # url of git server for origin of file
         # information about testcenter exams is a dict (of dicts), not a string,
         # so it cannot be easily exportable as a course element's attribute.
         'testcenter_info',
@@ -347,7 +347,7 @@ class XmlDescriptor(XModuleDescriptor):
         model_data['children'] = children
 
         model_data['xml_attributes'] = {}
-        model_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link
+        model_data['xml_attributes']['filename'] = definition.get('filename', ['', None])  # for git link
         for key, value in metadata.items():
             if key not in set(f.name for f in cls.fields + cls.lms.fields):
                 model_data['xml_attributes'][key] = value
@@ -409,7 +409,6 @@ class XmlDescriptor(XModuleDescriptor):
             # don't want e.g. data_dir
             if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
                 val = val_for_xml(attr)
-                #logging.debug('location.category = {0}, attr = {1}'.format(self.location.category, attr))
                 try:
                     xml_object.set(attr, val)
                 except Exception, e:

common/test/data/graphic_slider_tool/policies/2012_Fall.json

@@ -9,6 +9,6 @@
         "display_name": "Overview"
     },
     "graphical_slider_tool/sample_gst": {
-        "display_name": "Sample GST",
-    },
+        "display_name": "Sample GST"
+    }
 }

common/test/data/self_assessment/policies/2012_Fall.json

@@ -9,6 +9,6 @@
         "display_name": "Overview"
     },
     "selfassessment/SampleQuestion": {
-        "display_name": "Sample Question",
-    },
+        "display_name": "Sample Question"
+    }
 }

lms/djangoapps/courseware/access.py

@@ -523,10 +523,8 @@ def _adjust_start_date_for_beta_testers(user, descriptor):
     beta_group = course_beta_test_group_name(descriptor.location)
     if beta_group in user_groups:
         debug("Adjust start time: user in group %s", beta_group)
-        start_as_datetime = descriptor.lms.start
         delta = timedelta(descriptor.lms.days_early_for_beta)
-        effective = start_as_datetime - delta
-        # ...and back to time_struct
+        effective = descriptor.lms.start - delta
         return effective
 
     return descriptor.lms.start

lms/djangoapps/courseware/features/problems.py

@@ -2,8 +2,8 @@
 Steps for problem.feature lettuce tests
 '''
 
-#pylint: disable=C0111
-#pylint: disable=W0621
+# pylint: disable=C0111
+# pylint: disable=W0621
 
 from lettuce import world, step
 from lettuce.django import django_url
@@ -135,7 +135,7 @@ def action_button_present(_step, buttonname, doesnt_appear):
 
 
 @step(u'the button with the label "([^"]*)" does( not)? appear')
-def button_with_label_present(step, buttonname, doesnt_appear):
+def button_with_label_present(_step, buttonname, doesnt_appear):
     if doesnt_appear:
         assert world.browser.is_text_not_present(buttonname, wait_time=5)
     else:

lms/djangoapps/courseware/module_render.py

@@ -354,7 +354,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours
     system.set('position', position)
     system.set('DEBUG', settings.DEBUG)
     if settings.MITX_FEATURES.get('ENABLE_PSYCHOMETRICS'):
-        system.set('psychometrics_handler',		# set callback for updating PsychometricsData
+        system.set('psychometrics_handler',  # set callback for updating PsychometricsData
                    make_psychometrics_data_update_handler(course_id, user, descriptor.location.url()))
 
     try:

lms/djangoapps/courseware/tests/tests.py

@@ -65,7 +65,7 @@ def mongo_store_config(data_dir):
                 'db': 'test_xmodule',
                 'collection': 'modulestore_%s' % uuid4().hex,
                 'fs_root': data_dir,
-                'render_template': 'mitxmako.shortcuts.render_to_string'
+                'render_template': 'mitxmako.shortcuts.render_to_string',
             }
         }
     }

lms/envs/acceptance.py

@@ -24,7 +24,7 @@ modulestore_options = {
     'db': 'test_xmodule',
     'collection': 'acceptance_modulestore',
     'fs_root': TEST_ROOT / "data",
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

lms/envs/cms/dev.py

@@ -21,7 +21,7 @@ modulestore_options = {
     'db': 'xmodule',
     'collection': 'modulestore',
     'fs_root': DATA_DIR,
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

lms/envs/dev_mongo.py

@@ -19,7 +19,7 @@ MODULESTORE = {
             'db': 'xmodule',
             'collection': 'modulestore',
             'fs_root': GITHUB_REPO_ROOT,
-            'render_template': 'mitxmako.shortcuts.render_to_string'
+            'render_template': 'mitxmako.shortcuts.render_to_string',
         }
     }
 }