Fix pylint and pep8 errors

2013-04-12 08:53:04 -04:00
parent 9f3bad672a
commit bf6b886ccd
7 changed files with 95 additions and 70 deletions
--- a/.pylintrc
+++ b/.pylintrc
@@ -34,10 +34,13 @@ load-plugins=
 # multiple time (only on the command line, not in the configuration file where
 # it should appear only once).
 disable=
+# Never going to use these
 # C0301: Line too long
-# C0302: Too many lines in module
-# W0141: Used builtin function 'map'
 # W0142: Used * or ** magic
+# W0141: Used builtin function 'map'
+
+# Might use these when the code is in better shape
+# C0302: Too many lines in module
 # R0201: Method could be a function
 # R0901: Too many ancestors
 # R0902: Too many instance attributes
@@ -96,7 +99,18 @@ zope=no
 # List of members which are set dynamically and missed by pylint inference
 # system, and so shouldn't trigger E0201 when accessed. Python regular
 # expressions are accepted.
-generated-members=REQUEST,acl_users,aq_parent,objects,DoesNotExist,can_read,can_write,get_url,size,content
+generated-members=
+    REQUEST,
+    acl_users,
+    aq_parent,
+    objects,
+    DoesNotExist,
+    can_read,
+    can_write,
+    get_url,
+    size,
+    content,
+    status_code


 [BASIC]
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -49,6 +49,7 @@ class MongoCollectionFindWrapper(object):
        self.counter = self.counter+1
        return self.original(query, *args, **kwargs)

+
@override_settings(MODULESTORE=TEST_DATA_MODULESTORE)
 class ContentStoreToyCourseTest(ModuleStoreTestCase):
    """
@@ -188,27 +189,33 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
    def test_get_depth_with_drafts(self):
        import_from_xml(modulestore(), 'common/test/data/', ['simple'])

-        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
-                                                         'course', '2012_Fall', None]), depth=None)
+        course = modulestore('draft').get_item(
+            Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]),
+            depth=None
+        )

        # make sure no draft items have been returned
        num_drafts = self._get_draft_counts(course)
        self.assertEqual(num_drafts, 0)

-        problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
-                                                          'problem', 'ps01-simple', None]))
+        problem = modulestore('draft').get_item(
+            Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])
+        )

        # put into draft
        modulestore('draft').clone_item(problem.location, problem.location)

        # make sure we can query that item and verify that it is a draft
-        draft_problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
-                                                                'problem', 'ps01-simple', None]))
+        draft_problem = modulestore('draft').get_item(
+            Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])
+        )
        self.assertTrue(getattr(draft_problem, 'is_draft', False))

        #now requery with depth
-        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
-                                                         'course', '2012_Fall', None]), depth=None)
+        course = modulestore('draft').get_item(
+            Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]),
+            depth=None
+        )

        # make sure just one draft item have been returned
        num_drafts = self._get_draft_counts(course)
@@ -267,10 +274,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
        # make sure the parent no longer points to the child object which was deleted
        self.assertTrue(sequential.location.url() in chapter.children)

-        self.client.post(reverse('delete_item'),
-                         json.dumps({'id': sequential.location.url(), 'delete_children': 'true',
-                                     'delete_all_versions': 'true'}),
-                         "application/json")
+        self.client.post(
+            reverse('delete_item'),
+            json.dumps({'id': sequential.location.url(), 'delete_children': 'true', 'delete_all_versions': 'true'}),
+            "application/json"
+        )

        found = False
        try:
@@ -534,15 +542,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
        print 'Exporting to tempdir = {0}'.format(root_dir)

        # export out to a tempdir
-        exported = False
-        try:
-            export_to_xml(module_store, content_store, location, root_dir, 'test_export')
-            exported = True
-        except Exception:
-            print 'Exception thrown: {0}'.format(traceback.format_exc())
-            pass
-
-        self.assertTrue(exported)
+        export_to_xml(module_store, content_store, location, root_dir, 'test_export')


 class ContentStoreTest(ModuleStoreTestCase):
@@ -622,10 +622,12 @@ class ContentStoreTest(ModuleStoreTestCase):
        """Test viewing the index page with no courses"""
        # Create a course so there is something to view
        resp = self.client.get(reverse('index'))
-        self.assertContains(resp,
-                            '<h1 class="title-1">My Courses</h1>',
-                            status_code=200,
-                            html=True)
+        self.assertContains(
+            resp,
+            '<h1 class="title-1">My Courses</h1>',
+            status_code=200,
+            html=True
+        )

    def test_course_factory(self):
        """Test that the course factory works correctly."""
@@ -642,10 +644,12 @@ class ContentStoreTest(ModuleStoreTestCase):
        """Test viewing the index page with an existing course"""
        CourseFactory.create(display_name='Robot Super Educational Course')
        resp = self.client.get(reverse('index'))
-        self.assertContains(resp,
-                            '<span class="class-name">Robot Super Educational Course</span>',
-                            status_code=200,
-                            html=True)
+        self.assertContains(
+            resp,
+            '<span class="class-name">Robot Super Educational Course</span>',
+            status_code=200,
+            html=True
+        )

    def test_course_overview_view_with_course(self):
        """Test viewing the course overview page with an existing course"""
@@ -658,10 +662,12 @@ class ContentStoreTest(ModuleStoreTestCase):
        }

        resp = self.client.get(reverse('course_index', kwargs=data))
-        self.assertContains(resp,
-                            '<article class="courseware-overview" data-course-id="i4x://MITx/999/course/Robot_Super_Course">',
-                            status_code=200,
-                            html=True)
+        self.assertContains(
+            resp,
+            '<article class="courseware-overview" data-course-id="i4x://MITx/999/course/Robot_Super_Course">',
+            status_code=200,
+            html=True
+        )

    def test_clone_item(self):
        """Test cloning an item. E.g. creating a new section"""
@@ -677,7 +683,10 @@ class ContentStoreTest(ModuleStoreTestCase):

        self.assertEqual(resp.status_code, 200)
        data = parse_json(resp)
-        self.assertRegexpMatches(data['id'], '^i4x:\/\/MITx\/999\/chapter\/([0-9]|[a-f]){32}$')
+        self.assertRegexpMatches(
+            data['id'],
+            r"^i4x://MITx/999/chapter/([0-9]|[a-f]){32}$"
+        )

    def test_capa_module(self):
        """Test that a problem treats markdown specially."""
--- a/cms/djangoapps/contentstore/tests/test_course_settings.py
+++ b/cms/djangoapps/contentstore/tests/test_course_settings.py
@@ -47,12 +47,8 @@ class CourseTestCase(ModuleStoreTestCase):
        self.client = Client()
        self.client.login(username=uname, password=password)

-        t = 'i4x://edx/templates/course/Empty'
-        o = 'MITx'
-        n = '999'
-        dn = 'Robot Super Course'
-        self.course_location = Location('i4x', o, n, 'course', 'Robot_Super_Course')
-        CourseFactory.create(template=t, org=o, number=n, display_name=dn)
+        course = CourseFactory.create(template='i4x://edx/templates/course/Empty', org='MITx', number='999', display_name='Robot Super Course')
+        self.course_location = course.location


 class CourseDetailsTestCase(CourseTestCase):
@@ -86,17 +82,25 @@ class CourseDetailsTestCase(CourseTestCase):
        jsondetails = CourseDetails.fetch(self.course_location)
        jsondetails.syllabus = "<a href='foo'>bar</a>"
        # encode - decode to convert date fields and other data which changes form
-        self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).syllabus,
-                         jsondetails.syllabus, "After set syllabus")
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).syllabus,
+            jsondetails.syllabus, "After set syllabus"
+        )
        jsondetails.overview = "Overview"
-        self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).overview,
-                         jsondetails.overview, "After set overview")
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).overview,
+            jsondetails.overview, "After set overview"
+        )
        jsondetails.intro_video = "intro_video"
-        self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).intro_video,
-                         jsondetails.intro_video, "After set intro_video")
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).intro_video,
+            jsondetails.intro_video, "After set intro_video"
+        )
        jsondetails.effort = "effort"
-        self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).effort,
-                         jsondetails.effort, "After set effort")
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).effort,
+            jsondetails.effort, "After set effort"
+        )


 class CourseDetailsViewTest(CourseTestCase):
@@ -150,9 +154,7 @@ class CourseDetailsViewTest(CourseTestCase):

    @staticmethod
    def struct_to_datetime(struct_time):
-        return datetime.datetime(struct_time.tm_year, struct_time.tm_mon,
-                                 struct_time.tm_mday, struct_time.tm_hour,
-                                 struct_time.tm_min, struct_time.tm_sec, tzinfo=UTC())
+        return datetime.datetime(*struct_time[:6], tzinfo=UTC())

    def compare_date_fields(self, details, encoded, context, field):
        if details[field] is not None:
@@ -271,18 +273,20 @@ class CourseMetadataEditingTest(CourseTestCase):
        self.assertIn('xqa_key', test_model, 'xqa_key field ')

    def test_update_from_json(self):
-        test_model = CourseMetadata.update_from_json(self.course_location,
-                                                     {"advertised_start": "start A",
-                                                      "testcenter_info": {"c": "test"},
-                                                      "days_early_for_beta": 2})
+        test_model = CourseMetadata.update_from_json(self.course_location, {
+            "advertised_start": "start A",
+            "testcenter_info": {"c": "test"},
+            "days_early_for_beta": 2
+        })
        self.update_check(test_model)
        # try fresh fetch to ensure persistence
        test_model = CourseMetadata.fetch(self.course_location)
        self.update_check(test_model)
        # now change some of the existing metadata
-        test_model = CourseMetadata.update_from_json(self.course_location,
-                                                     {"advertised_start": "start B",
-                                                      "display_name": "jolly roger"})
+        test_model = CourseMetadata.update_from_json(self.course_location, {
+            "advertised_start": "start B",
+            "display_name": "jolly roger"}
+        )
        self.assertIn('display_name', test_model, 'Missing editable metadata field')
        self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value")
        self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field')
--- a/cms/djangoapps/contentstore/tests/test_utils.py
+++ b/cms/djangoapps/contentstore/tests/test_utils.py
@@ -30,7 +30,7 @@ class LMSLinksTestCase(TestCase):

 class UrlReverseTestCase(ModuleStoreTestCase):
    """ Tests for get_url_reverse """
-    def test_CoursePageNames(self):
+    def test_course_page_names(self):
        """ Test the defined course pages. """
        course = CourseFactory.create(org='mitX', number='666', display_name='URL Reverse Course')

@@ -70,4 +70,3 @@ class UrlReverseTestCase(ModuleStoreTestCase):
            'https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about',
            utils.get_url_reverse('https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about', course)
        )
-        
--- a/cms/djangoapps/contentstore/tests/utils.py
+++ b/cms/djangoapps/contentstore/tests/utils.py
@@ -2,8 +2,6 @@
 Utilities for contentstore tests
 '''

-#pylint: disable=W0603
-
 import json

 from student.models import Registration
--- a/cms/envs/test.py
+++ b/cms/envs/test.py
@@ -28,7 +28,7 @@ GITHUB_REPO_ROOT = TEST_ROOT / "data"
 COMMON_TEST_DATA_ROOT = COMMON_ROOT / "test" / "data"

 # Makes the tests run much faster...
-SOUTH_TESTS_MIGRATE = False # To disable migrations and use syncdb instead
+SOUTH_TESTS_MIGRATE = False  # To disable migrations and use syncdb instead

 # TODO (cpennington): We need to figure out how envs/test.py can inject things into common.py so that we don't have to repeat this sort of thing
 STATICFILES_DIRS = [
@@ -41,7 +41,7 @@ STATICFILES_DIRS += [
    if os.path.isdir(COMMON_TEST_DATA_ROOT / course_dir)
 ]

-modulestore_options = {
+MODULESTORE_OPTIONS = {
    'default_class': 'xmodule.raw_module.RawDescriptor',
    'host': 'localhost',
    'db': 'test_xmodule',
@@ -53,15 +53,15 @@ modulestore_options = {
 MODULESTORE = {
    'default': {
        'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
-        'OPTIONS': modulestore_options
+        'OPTIONS': MODULESTORE_OPTIONS
    },
    'direct': {
        'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
-        'OPTIONS': modulestore_options
+        'OPTIONS': MODULESTORE_OPTIONS
    },
    'draft': {
        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
-        'OPTIONS': modulestore_options
+        'OPTIONS': MODULESTORE_OPTIONS
    }
 }

--- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
@@ -7,6 +7,7 @@ from django.conf import settings
 import xmodule.modulestore.django
 from xmodule.templates import update_templates

+
 class ModuleStoreTestCase(TestCase):
    """ Subclass for any test case that uses the mongodb
    module store. This populates a uniquely named modulestore