Merge pull request #542 from edx/db/fix-pylint

Fix some pylint issues

cms/djangoapps/contentstore/management/commands/clone.py

-###
-### Script for cloning a course
-###
+"""
+Script for cloning a course
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.django import modulestore
@@ -15,23 +15,25 @@ from auth.authz import _copy_course_group
 
 
 class Command(BaseCommand):
+    """Clone a MongoDB-backed course to another location"""
     help = 'Clone a MongoDB backed course to another location'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 2:
             raise CommandError("clone requires two arguments: <source-location> <dest-location>")
 
         source_location_str = args[0]
         dest_location_str = args[1]
 
-        ms = modulestore('direct')
-        cs = contentstore()
+        mstore = modulestore('direct')
+        cstore = contentstore()
 
-        print "Cloning course {0} to {1}".format(source_location_str, dest_location_str)
+        print("Cloning course {0} to {1}".format(source_location_str, dest_location_str))
 
         source_location = CourseDescriptor.id_to_location(source_location_str)
         dest_location = CourseDescriptor.id_to_location(dest_location_str)
 
-        if clone_course(ms, cs, source_location, dest_location):
-            print "copying User permissions..."
+        if clone_course(mstore, cstore, source_location, dest_location):
+            print("copying User permissions...")
             _copy_course_group(source_location, dest_location)

cms/djangoapps/contentstore/management/commands/dump_course_structure.py

+"""
+Script for dumping course dumping the course structure
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
@@ -9,10 +12,14 @@ filter_list = ['xml_attributes', 'checklists']
 
 
 class Command(BaseCommand):
+    """
+    The Django command for dumping course structure
+    """
     help = '''Write out to stdout a structural and metadata information about a course in a flat dictionary serialized
               in a JSON format. This can be used for analytics.'''
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) < 2 or len(args) > 3:
             raise CommandError("dump_course_structure requires two or more arguments: <location> <outfile> |<db>|")
 
@@ -32,7 +39,7 @@ class Command(BaseCommand):
         try:
             course = store.get_item(loc, depth=4)
         except:
-            print 'Could not find course at {0}'.format(course_id)
+            print('Could not find course at {0}'.format(course_id))
             return
 
         info = {}

cms/djangoapps/contentstore/management/commands/export.py

-###
-### Script for exporting courseware from Mongo to a tar.gz file
-###
+"""
+Script for exporting courseware from Mongo to a tar.gz file
+"""
 import os
 
 from django.core.management.base import BaseCommand, CommandError
@@ -10,20 +10,21 @@ from xmodule.contentstore.django import contentstore
 from xmodule.course_module import CourseDescriptor
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """
+    Export the specified data directory into the default ModuleStore
+    """
     help = 'Export the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 2:
             raise CommandError("export requires two arguments: <course location> <output path>")
 
         course_id = args[0]
         output_path = args[1]
 
-        print "Exporting course id = {0} to {1}".format(course_id, output_path)
+        print("Exporting course id = {0} to {1}".format(course_id, output_path))
 
         location = CourseDescriptor.id_to_location(course_id)
 

cms/djangoapps/contentstore/management/commands/export_all_courses.py

-###
-### Script for exporting all courseware from Mongo to a directory
-###
-import os
-
+"""
+Script for exporting all courseware from Mongo to a directory
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.django import modulestore
@@ -10,13 +8,12 @@ from xmodule.contentstore.django import contentstore
 from xmodule.course_module import CourseDescriptor
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """Export all courses from mongo to the specified data directory"""
     help = 'Export all courses from mongo to the specified data directory'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 1:
             raise CommandError("export requires one argument: <output path>")
 
@@ -27,14 +24,14 @@ class Command(BaseCommand):
         root_dir = output_path
         courses = ms.get_courses()
 
-        print "%d courses to export:" % len(courses)
+        print("%d courses to export:" % len(courses))
         cids = [x.id for x in courses]
-        print cids
+        print(cids)
 
         for course_id in cids:
 
-            print "-"*77
-            print "Exporting course id = {0} to {1}".format(course_id, output_path)
+            print("-"*77)
+            print("Exporting course id = {0} to {1}".format(course_id, output_path))
 
             if 1:
                 try:
@@ -42,6 +39,6 @@ class Command(BaseCommand):
                     course_dir = course_id.replace('/', '...')
                     export_to_xml(ms, cs, location, root_dir, course_dir, modulestore())
                 except Exception as err:
-                    print "="*30 + "> Oops, failed to export %s" % course_id
-                    print "Error:"
-                    print err
+                    print("="*30 + "> Oops, failed to export %s" % course_id)
+                    print("Error:")
+                    print(err)

cms/djangoapps/contentstore/management/commands/import.py

-###
-### Script for importing courseware from XML format
-###
+"""
+Script for importing courseware from XML format
+"""
 
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import import_from_xml
@@ -8,13 +8,14 @@ from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """
+    Import the specified data directory into the default ModuleStore
+    """
     help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")
 
@@ -23,8 +24,8 @@ class Command(BaseCommand):
             course_dirs = args[1:]
         else:
             course_dirs = None
-        print "Importing.  Data_dir={data}, course_dirs={courses}".format(
+        print("Importing.  Data_dir={data}, course_dirs={courses}".format(
             data=data_dir,
-            courses=course_dirs)
+            courses=course_dirs))
         import_from_xml(modulestore('direct'), data_dir, course_dirs, load_error_modules=False,
                         static_content_store=contentstore(), verbose=True)

cms/djangoapps/contentstore/management/commands/xlint.py

+"""
+Verify the structure of courseware as to it's suitability for import
+To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import perform_xlint
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
-    help = \
-        '''
-        Verify the structure of courseware as to it's suitability for import
-        To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
-        '''
+    """Verify the structure of courseware as to it's suitability for import"""
+    help = "Verify the structure of courseware as to it's suitability for import"
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")
 
@@ -21,7 +20,7 @@ class Command(BaseCommand):
             course_dirs = args[1:]
         else:
             course_dirs = None
-        print "Importing.  Data_dir={data}, course_dirs={courses}".format(
+        print("Importing.  Data_dir={data}, course_dirs={courses}".format(
             data=data_dir,
-            courses=course_dirs)
+            courses=course_dirs))
         perform_xlint(data_dir, course_dirs, load_error_modules=False)

cms/djangoapps/contentstore/tests/test_assets.py

@@ -50,9 +50,9 @@ class UploadTestCase(CourseTestCase):
 
     @skip("CorruptGridFile error on continuous integration server")
     def test_happy_path(self):
-        file = BytesIO("sample content")
-        file.name = "sample.txt"
-        resp = self.client.post(self.url, {"name": "my-name", "file": file})
+        f = BytesIO("sample content")
+        f.name = "sample.txt"
+        resp = self.client.post(self.url, {"name": "my-name", "file": f})
         self.assert2XX(resp.status_code)
 
     def test_no_file(self):

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -27,6 +27,7 @@ class ChecklistTestCase(CourseTestCase):
         """
         self.assertEqual(persisted['short_description'], request['short_description'])
         compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded'))
+        pers, req = None, None
         for pers, req in zip(persisted['items'], request['items']):
             self.assertEqual(pers['short_description'], req['short_description'])
         self.assertEqual(pers['long_description'], req['long_description'])

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -95,8 +95,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.client.login(username=uname, password=password)
 
     def tearDown(self):
-        mongo = MongoClient()
-        mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
+        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
         _CONTENTSTORE.clear()
 
     def check_components_on_page(self, component_types, expected_types):

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -18,8 +18,6 @@ from xmodule.modulestore.tests.factories import CourseFactory
 
 
 from models.settings.course_metadata import CourseMetadata
-from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.modulestore.django import modulestore
 from xmodule.fields import Date
 
 from .utils import CourseTestCase
@@ -167,8 +165,8 @@ class CourseDetailsViewTest(CourseTestCase):
         self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val))
 
     @staticmethod
-    def convert_datetime_to_iso(dt):
-        return Date().to_json(dt)
+    def convert_datetime_to_iso(datetime_obj):
+        return Date().to_json(datetime_obj)
 
     def test_update_and_fetch(self):
         loc = self.course.location

cms/djangoapps/contentstore/tests/test_i18n.py

@@ -85,9 +85,11 @@ class InternationalizationTest(ModuleStoreTestCase):
                                HTTP_ACCEPT_LANGUAGE='fr'
                                )
 
-        TEST_STRING = u'<h1 class="title-1">' \
-                      + u'My \xc7\xf6\xfcrs\xe9s L#' \
-                      + u'</h1>'
+        TEST_STRING = (
+            u'<h1 class="title-1">'
+            u'My \xc7\xf6\xfcrs\xe9s L#'
+            u'</h1>'
+        )
 
         self.assertContains(resp,
                             TEST_STRING,

cms/djangoapps/contentstore/tests/test_item.py

@@ -14,19 +14,26 @@ class DeleteItem(CourseTestCase):
         super(DeleteItem, self).setUp()
         self.course = CourseFactory.create(org='mitX', number='333', display_name='Dummy Course')
 
-    def testDeleteStaticPage(self):
+    def test_delete_static_page(self):
         # Add static tab
         data = json.dumps({
             'parent_location': 'i4x://mitX/333/course/Dummy_Course',
             'category': 'static_tab'
         })
 
-        resp = self.client.post(reverse('create_item'), data,
-            content_type="application/json")
+        resp = self.client.post(
+            reverse('create_item'),
+            data,
+            content_type="application/json"
+        )
         self.assertEqual(resp.status_code, 200)
 
         # Now delete it. There was a bug that the delete was failing (static tabs do not exist in draft modulestore).
-        resp = self.client.post(reverse('delete_item'), resp.content, "application/json")
+        resp = self.client.post(
+            reverse('delete_item'),
+            resp.content,
+            "application/json"
+        )
         self.assertEqual(resp.status_code, 200)
 
 
@@ -122,6 +129,7 @@ class TestCreateItem(CourseTestCase):
         )
         self.assertEqual(resp.status_code, 200)
 
+
 class TestEditItem(CourseTestCase):
     """
     Test contentstore.views.item.save_item
@@ -151,10 +159,10 @@ class TestEditItem(CourseTestCase):
         chap_location = self.response_id(resp)
         resp = self.client.post(
             reverse('create_item'),
-            json.dumps(
-                {'parent_location': chap_location,
-                 'category': 'sequential'
-                }),
+            json.dumps({
+                'parent_location': chap_location,
+                'category': 'sequential',
+            }),
             content_type="application/json"
         )
         self.seq_location = self.response_id(resp)
@@ -162,9 +170,10 @@ class TestEditItem(CourseTestCase):
         template_id = 'multiplechoice.yaml'
         resp = self.client.post(
             reverse('create_item'),
-            json.dumps({'parent_location': self.seq_location,
-             'category': 'problem',
-             'boilerplate': template_id
+            json.dumps({
+                'parent_location': self.seq_location,
+                'category': 'problem',
+                'boilerplate': template_id,
             }),
             content_type="application/json"
         )
@@ -195,7 +204,6 @@ class TestEditItem(CourseTestCase):
         problem = modulestore('draft').get_item(self.problems[0])
         self.assertEqual(problem.rerandomize, 'never')
 
-
     def test_null_field(self):
         """
         Sending null in for a field 'deletes' it
@@ -240,4 +248,3 @@ class TestEditItem(CourseTestCase):
         sequential = modulestore().get_item(self.seq_location)
         self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
         self.assertEqual(sequential.lms.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC))
-

cms/djangoapps/contentstore/tests/tests.py

@@ -15,14 +15,16 @@ class ContentStoreTestCase(ModuleStoreTestCase):
         Login.  View should always return 200.  The success/fail is in the
         returned json
         """
-        resp = self.client.post(reverse('login_post'),
-                                {'email': email, 'password': password})
+        resp = self.client.post(
+            reverse('login_post'),
+            {'email': email, 'password': password}
+        )
         self.assertEqual(resp.status_code, 200)
         return resp
 
-    def login(self, email, pw):
+    def login(self, email, password):
         """Login, check that it worked."""
-        resp = self._login(email, pw)
+        resp = self._login(email, password)
         data = parse_json(resp)
         self.assertTrue(data['success'])
         return resp
@@ -178,11 +180,15 @@ class ForumTestCase(CourseTestCase):
 
     def test_blackouts(self):
         now = datetime.datetime.now(UTC)
-        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in
-            [(now - datetime.timedelta(days=14), now - datetime.timedelta(days=11)),
-                (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))]]
+        times1 = [
+            (now - datetime.timedelta(days=14), now - datetime.timedelta(days=11)),
+            (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))
+        ]
+        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in times1]
         self.assertTrue(self.course.forum_posts_allowed)
-        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in
-            [(now - datetime.timedelta(days=14), now + datetime.timedelta(days=2)),
-                (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))]]
+        times2 = [
+            (now - datetime.timedelta(days=14), now + datetime.timedelta(days=2)),
+            (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))
+        ]
+        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in times2]
         self.assertFalse(self.course.forum_posts_allowed)

cms/djangoapps/contentstore/views/assets.py

@@ -283,7 +283,7 @@ def import_course(request, org, course, name):
         tar_file.extractall(course_dir + '/')
 
         # find the 'course.xml' file
-
+        dirpath = None
         for dirpath, _dirnames, filenames in os.walk(course_dir):
             for filename in filenames:
                 if filename == 'course.xml':

cms/djangoapps/contentstore/views/component.py

@@ -58,6 +58,7 @@ ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules'
 
 @login_required
 def edit_subsection(request, location):
+    "Edit the subsection of a course"
     # check that we have permissions to edit this item
     try:
         course = get_course_for_item(location)
@@ -269,6 +270,7 @@ def assignment_type_update(request, org, course, category, name):
 @login_required
 @expect_json
 def create_draft(request):
+    "Create a draft"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -285,6 +287,7 @@ def create_draft(request):
 @login_required
 @expect_json
 def publish_draft(request):
+    "Publish a draft"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -300,6 +303,7 @@ def publish_draft(request):
 @login_required
 @expect_json
 def unpublish_unit(request):
+    "Unpublish a unit"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -317,6 +321,7 @@ def unpublish_unit(request):
 @login_required
 @ensure_csrf_cookie
 def module_info(request, module_location):
+    "Get or set information for a module in the modulestore"
     location = Location(module_location)
 
     # check that logged in user has permissions to this item

cms/djangoapps/contentstore/views/preview.py

@@ -68,6 +68,7 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
 
 @login_required
 def preview_component(request, location):
+    "Return the HTML preview of a component"
     # TODO (vshnayder): change name from id to location in coffee+html as well.
     if not has_access(request.user, location):
         return HttpResponseForbidden()
@@ -91,6 +92,7 @@ def preview_module_system(request, preview_id, descriptor):
     """
 
     def preview_model_data(descriptor):
+        "Helper method to create a DbModel from a descriptor"
         return DbModel(
             SessionKeyValueStore(request, descriptor._model_data),
             descriptor.module_class,

cms/djangoapps/contentstore/views/public.py

+"""
+Public views
+"""
 from django_future.csrf import ensure_csrf_cookie
 from django.core.context_processors import csrf
 from django.shortcuts import redirect
@@ -10,10 +13,6 @@ from .user import index
 
 __all__ = ['signup', 'old_login_redirect', 'login_page', 'howitworks']
 
-"""
-Public views
-"""
-
 
 @ensure_csrf_cookie
 def signup(request):
@@ -45,6 +44,7 @@ def login_page(request):
 
 
 def howitworks(request):
+    "Proxy view"
     if request.user.is_authenticated():
         return index(request)
     else:

cms/djangoapps/contentstore/views/tabs.py

+"""
+Views related to course tabs
+"""
 from access import has_access
 from util.json_request import expect_json
 
@@ -39,6 +42,7 @@ def initialize_course_tabs(course):
 @login_required
 @expect_json
 def reorder_static_tabs(request):
+    "Order the static tabs in the requested order"
     tabs = request.POST['tabs']
     course = get_course_for_item(tabs[0])
 
@@ -86,6 +90,7 @@ def reorder_static_tabs(request):
 @login_required
 @ensure_csrf_cookie
 def edit_tabs(request, org, course, coursename):
+    "Edit tabs"
     location = ['i4x', org, course, 'course', coursename]
     store = get_modulestore(location)
     course_item = store.get_item(location)
@@ -122,6 +127,7 @@ def edit_tabs(request, org, course, coursename):
 @login_required
 @ensure_csrf_cookie
 def static_pages(request, org, course, coursename):
+    "Static pages view"
 
     location = get_location_and_verify_access(request, org, course, coursename)
 

pylintrc

@@ -160,7 +160,7 @@ variable-rgx=[a-z_][a-z0-9_]{2,30}$
 inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
 
 # Good variable names which should always be accepted, separated by a comma
-good-names=i,j,k,ex,Run,_
+good-names=f,i,j,k,ex,Run,_,__
 
 # Bad variable names which should always be refused, separated by a comma
 bad-names=foo,bar,baz,toto,tutu,tata