Quality cleanup

common/djangoapps/student/models.py

@@ -51,10 +51,10 @@ from ratelimitbackend import admin
 
 import analytics
 
-unenroll_done = Signal(providing_args=["course_enrollment"])
+UNENROLL_DONE = Signal(providing_args=["course_enrollment"])
 log = logging.getLogger(__name__)
 AUDIT_LOG = logging.getLogger("audit")
-SessionStore = import_module(settings.SESSION_ENGINE).SessionStore
+SessionStore = import_module(settings.SESSION_ENGINE).SessionStore  # pylint: disable=invalid-name
 
 
 class AnonymousUserId(models.Model):
@@ -133,7 +133,7 @@ def anonymous_id_for_user(user, course_id, save=True):
     return digest
 
 
-def user_by_anonymous_id(id):
+def user_by_anonymous_id(uid):
     """
     Return user by anonymous_user_id using AnonymousUserId lookup table.
 
@@ -142,11 +142,11 @@ def user_by_anonymous_id(id):
     because this function will be used inside xmodule w/o django access.
     """
 
-    if id is None:
+    if uid is None:
         return None
 
     try:
-        return User.objects.get(anonymoususerid__anonymous_user_id=id)
+        return User.objects.get(anonymoususerid__anonymous_user_id=uid)
     except ObjectDoesNotExist:
         return None
 
@@ -192,7 +192,7 @@ class UserProfile(models.Model):
     MITx fall prototype.
     """
 
-    class Meta:
+    class Meta:  # pylint: disable=missing-docstring
         db_table = "auth_userprofile"
 
     # CRITICAL TODO/SECURITY
@@ -250,7 +250,7 @@ class UserProfile(models.Model):
     goals = models.TextField(blank=True, null=True)
     allow_certificate = models.BooleanField(default=1)
 
-    def get_meta(self):
+    def get_meta(self):  # pylint: disable=missing-docstring
         js_str = self.meta
         if not js_str:
             js_str = dict()
@@ -259,8 +259,8 @@ class UserProfile(models.Model):
 
         return js_str
 
-    def set_meta(self, js):
-        self.meta = json.dumps(js)
+    def set_meta(self, meta_json):  # pylint: disable=missing-docstring
+        self.meta = json.dumps(meta_json)
 
     def set_login_session(self, session_id=None):
         """
@@ -723,7 +723,7 @@ class CourseEnrollment(models.Model):
                 )
 
             else:
-                unenroll_done.send(sender=None, course_enrollment=self)
+                UNENROLL_DONE.send(sender=None, course_enrollment=self)
 
                 self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
 
@@ -961,12 +961,12 @@ class CourseEnrollment(models.Model):
         # Unfortunately, Django's "group by"-style queries look super-awkward
         query = use_read_replica_if_available(cls.objects.filter(course_id=course_id, is_active=True).values('mode').order_by().annotate(Count('mode')))
         total = 0
-        d = defaultdict(int)
+        enroll_dict = defaultdict(int)
         for item in query:
-            d[item['mode']] = item['mode__count']
+            enroll_dict[item['mode']] = item['mode__count']
             total += item['mode__count']
-        d['total'] = total
-        return d
+        enroll_dict['total'] = total
+        return enroll_dict
 
     def is_paid_course(self):
         """
@@ -999,7 +999,7 @@ class CourseEnrollment(models.Model):
         a verified certificate and the deadline for refunds has not yet passed.
         """
         # In order to support manual refunds past the deadline, set can_refund on this object.
-        # On unenrolling, the "unenroll_done" signal calls CertificateItem.refund_cert_callback(),
+        # On unenrolling, the "UNENROLL_DONE" signal calls CertificateItem.refund_cert_callback(),
         # which calls this method to determine whether to refund the order.
         # This can't be set directly because refunds currently happen as a side-effect of unenrolling.
         # (side-effects are bad)
@@ -1031,7 +1031,7 @@ class CourseEnrollmentAllowed(models.Model):
 
     created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
 
-    class Meta:
+    class Meta:  # pylint: disable=missing-docstring
         unique_together = (('email', 'course_id'),)
 
     def __unicode__(self):
@@ -1055,7 +1055,7 @@ class CourseAccessRole(models.Model):
     course_id = CourseKeyField(max_length=255, db_index=True, blank=True)
     role = models.CharField(max_length=64, db_index=True)
 
-    class Meta:
+    class Meta:  # pylint: disable=missing-docstring
         unique_together = ('user', 'org', 'course_id', 'role')
 
     @property
@@ -1071,7 +1071,7 @@ class CourseAccessRole(models.Model):
         Overriding eq b/c the django impl relies on the primary key which requires fetch. sometimes we
         just want to compare roles w/o doing another fetch.
         """
-        return type(self) == type(other) and self._key == other._key
+        return type(self) == type(other) and self._key == other._key  # pylint: disable=protected-access
 
     def __hash__(self):
         return hash(self._key)
@@ -1080,7 +1080,7 @@ class CourseAccessRole(models.Model):
         """
         Lexigraphic sort
         """
-        return self._key < other._key
+        return self._key < other._key  # pylint: disable=protected-access
 
     def __unicode__(self):
         return "[CourseAccessRole] user: {}   role: {}   org: {}   course: {}".format(self.user.username, self.role, self.org, self.course_id)
@@ -1107,32 +1107,32 @@ def get_user_by_username_or_email(username_or_email):
 
 
 def get_user(email):
-    u = User.objects.get(email=email)
-    up = UserProfile.objects.get(user=u)
-    return u, up
+    user = User.objects.get(email=email)
+    u_prof = UserProfile.objects.get(user=user)
+    return user, u_prof
 
 
 def user_info(email):
-    u, up = get_user(email)
-    print "User id", u.id
-    print "Username", u.username
-    print "E-mail", u.email
-    print "Name", up.name
-    print "Location", up.location
-    print "Language", up.language
-    return u, up
+    user, u_prof = get_user(email)
+    print "User id", user.id
+    print "Username", user.username
+    print "E-mail", user.email
+    print "Name", u_prof.name
+    print "Location", u_prof.location
+    print "Language", u_prof.language
+    return user, u_prof
 
 
 def change_email(old_email, new_email):
-    u = User.objects.get(email=old_email)
-    u.email = new_email
-    u.save()
+    user = User.objects.get(email=old_email)
+    user.email = new_email
+    user.save()
 
 
 def change_name(email, new_name):
-    u, up = get_user(email)
-    up.name = new_name
-    up.save()
+    _user, u_prof = get_user(email)
+    u_prof.name = new_name
+    u_prof.save()
 
 
 def user_count():
@@ -1163,10 +1163,12 @@ def remove_user_from_group(user, group):
     utg.users.remove(User.objects.get(username=user))
     utg.save()
 
-default_groups = {'email_future_courses': 'Receive e-mails about future MITx courses',
+DEFAULT_GROUPS = {
+    'email_future_courses': 'Receive e-mails about future MITx courses',
     'email_helpers': 'Receive e-mails about how to help with MITx',
     'mitx_unenroll': 'Fully unenrolled -- no further communications',
-                  '6002x_unenroll': 'Took and dropped 6002x'}
+    '6002x_unenroll': 'Took and dropped 6002x'
+}
 
 
 def add_user_to_default_group(user, group):
@@ -1175,7 +1177,7 @@ def add_user_to_default_group(user, group):
     except UserTestGroup.DoesNotExist:
         utg = UserTestGroup()
         utg.name = group
-        utg.description = default_groups[group]
+        utg.description = DEFAULT_GROUPS[group]
         utg.save()
     utg.users.add(User.objects.get(username=user))
     utg.save()
@@ -1188,11 +1190,12 @@ def create_comments_service_user(user):
     try:
         cc_user = cc.User.from_django_user(user)
         cc_user.save()
-    except Exception as e:
-        log = logging.getLogger("edx.discussion")
+    except Exception:  # pylint: disable=broad-except
+        log = logging.getLogger("edx.discussion")  # pylint: disable=redefined-outer-name
         log.error(
             "Could not create comments service user with id {}".format(user.id),
-            exc_info=True)
+            exc_info=True
+        )
 
 # Define login and logout handlers here in the models file, instead of the views file,
 # so that they are more likely to be loaded when a Studio user brings up the Studio admin
@@ -1201,7 +1204,7 @@ def create_comments_service_user(user):
 
 
 @receiver(user_logged_in)
-def log_successful_login(sender, request, user, **kwargs):
+def log_successful_login(sender, request, user, **kwargs):  # pylint: disable=unused-argument
     """Handler to log when logins have occurred successfully."""
     if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
         AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id))
@@ -1210,7 +1213,7 @@ def log_successful_login(sender, request, user, **kwargs):
 
 
 @receiver(user_logged_out)
-def log_successful_logout(sender, request, user, **kwargs):
+def log_successful_logout(sender, request, user, **kwargs):  # pylint: disable=unused-argument
     """Handler to log when logouts have occurred successfully."""
     if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
         AUDIT_LOG.info(u"Logout - user.id: {0}".format(request.user.id))
@@ -1220,7 +1223,7 @@ def log_successful_logout(sender, request, user, **kwargs):
 
 @receiver(user_logged_in)
 @receiver(user_logged_out)
-def enforce_single_login(sender, request, user, signal, **kwargs):
+def enforce_single_login(sender, request, user, signal, **kwargs):    # pylint: disable=unused-argument
     """
     Sets the current session id in the user profile,
     to prevent concurrent logins.

common/lib/capa/capa/tests/test_html_render.py

@@ -26,8 +26,9 @@ class CapaHtmlRenderTest(unittest.TestCase):
         problem = new_loncapa_problem(xml_str)
 
         # Render the HTML
-        rendered_html = etree.XML(problem.get_html())
+        etree.XML(problem.get_html())
         # expect that we made it here without blowing up
+        self.assertTrue(True)
 
     def test_include_html(self):
         # Create a test file to include
@@ -123,17 +124,17 @@ class CapaHtmlRenderTest(unittest.TestCase):
         # Render the HTML
         rendered_html = etree.XML(problem.get_html())
 
-
         # expect the javascript is still present in the rendered html
         self.assertTrue("<script type=\"text/javascript\">function(){}</script>" in etree.tostring(rendered_html))
 
-
     def test_render_response_xml(self):
         # Generate some XML for a string response
-        kwargs = {'question_text': "Test question",
+        kwargs = {
+            'question_text': "Test question",
             'explanation_text': "Test explanation",
             'answer': 'Test answer',
-                    'hints': [('test prompt', 'test_hint', 'test hint text')]}
+            'hints': [('test prompt', 'test_hint', 'test hint text')]
+        }
         xml_str = StringResponseXMLFactory().build_xml(**kwargs)
 
         # Mock out the template renderer
@@ -186,18 +187,21 @@ class CapaHtmlRenderTest(unittest.TestCase):
 
         expected_solution_context = {'id': '1_solution_1'}
 
-        expected_calls = [mock.call('textline.html', expected_textline_context),
+        expected_calls = [
+            mock.call('textline.html', expected_textline_context),
             mock.call('solutionspan.html', expected_solution_context),
             mock.call('textline.html', expected_textline_context),
-                mock.call('solutionspan.html', expected_solution_context)]
-
-        self.assertEqual(the_system.render_template.call_args_list,
-                            expected_calls)
+            mock.call('solutionspan.html', expected_solution_context)
+        ]
 
+        self.assertEqual(
+            the_system.render_template.call_args_list,
+            expected_calls
+        )
 
     def test_render_response_with_overall_msg(self):
         # CustomResponse script that sets an overall_message
-        script=textwrap.dedent("""
+        script = textwrap.dedent("""
             def check_func(*args):
                 msg = '<p>Test message 1<br /></p><p>Test message 2</p>'
                 return {'overall_message': msg,
@@ -205,19 +209,18 @@ class CapaHtmlRenderTest(unittest.TestCase):
         """)
 
         # Generate some XML for a CustomResponse
-        kwargs = {'script':script, 'cfn': 'check_func'}
+        kwargs = {'script': script, 'cfn': 'check_func'}
         xml_str = CustomResponseXMLFactory().build_xml(**kwargs)
 
         # Create the problem and render the html
         problem = new_loncapa_problem(xml_str)
 
         # Grade the problem
-        correctmap = problem.grade_answers({'1_2_1': 'test'})
+        problem.grade_answers({'1_2_1': 'test'})
 
         # Render the html
         rendered_html = etree.XML(problem.get_html())
 
-
         # Expect that there is a <div> within the response <div>
         # with css class response_message
         msg_div_element = rendered_html.find(".//div[@class='response_message']")
@@ -232,7 +235,6 @@ class CapaHtmlRenderTest(unittest.TestCase):
         self.assertEqual(msg_p_elements[1].tag, "p")
         self.assertEqual(msg_p_elements[1].text, "Test message 2")
 
-
     def test_substitute_python_vars(self):
         # Generate some XML with Python variables defined in a script
         # and used later as attributes

common/lib/i18n/tests/test_extract_and_generate.py

+"""
+This test tests that i18n extraction (`paver i18n_extract -v`) works properly.
+"""
 from datetime import datetime, timedelta
 import os
 import sys
-import string
+import string  # pylint: disable=deprecated-module
 import random
 import re
 
@@ -65,12 +68,14 @@ class TestGenerate(TestCase):
         generate.main(verbosity=0, strict=False)
         for locale in CONFIGURATION.translated_locales:
             for filename in ('django', 'djangojs'):
-                mofile = filename+'.mo'
+                mofile = filename + '.mo'
                 path = os.path.join(CONFIGURATION.get_messages_dir(locale), mofile)
                 exists = os.path.exists(path)
                 self.assertTrue(exists, msg='Missing file in locale %s: %s' % (locale, mofile))
-                self.assertTrue(datetime.fromtimestamp(os.path.getmtime(path), UTC) >= self.start_time,
-                                msg='File not recently modified: %s' % path)
+                self.assertTrue(
+                    datetime.fromtimestamp(os.path.getmtime(path), UTC) >= self.start_time,
+                    msg='File not recently modified: %s' % path
+                )
             # Segmenting means that the merge headers don't work they way they
             # used to, so don't make this check for now. I'm not sure if we'll
             # get the merge header back eventually, or delete this code eventually.
@@ -88,12 +93,14 @@ class TestGenerate(TestCase):
 
         """
         path = os.path.join(CONFIGURATION.get_messages_dir(locale), 'django.po')
-        po = pofile(path)
+        pof = pofile(path)
         pattern = re.compile('^#-#-#-#-#', re.M)
-        match = pattern.findall(po.header)
-        self.assertEqual(len(match), 3,
-                         msg="Found %s (should be 3) merge comments in the header for %s" % \
-                         (len(match), path))
+        match = pattern.findall(pof.header)
+        self.assertEqual(
+            len(match),
+            3,
+            msg="Found %s (should be 3) merge comments in the header for %s" % (len(match), path)
+        )
 
 
 def random_name(size=6):

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

@@ -134,8 +134,6 @@ def xml_store_config(data_dir):
     return store
 
 
-
-
 class ModuleStoreTestCase(TestCase):
     """
     Subclass for any test case that uses a ModuleStore.
@@ -282,19 +280,23 @@ class ModuleStoreTestCase(TestCase):
         # Call superclass implementation
         super(ModuleStoreTestCase, self)._post_teardown()
 
-    def create_sample_course(self, org, course, run, block_info_tree=default_block_info_tree, course_fields=None):
+    def create_sample_course(self, org, course, run, block_info_tree=None, course_fields=None):
         """
         create a course in the default modulestore from the collection of BlockInfo
         records defining the course tree
         Returns:
             course_loc: the CourseKey for the created course
         """
+        if block_info_tree is None:
+            block_info_tree = default_block_info_tree
+
         with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, None):
 #             with self.store.bulk_operations(self.store.make_course_key(org, course, run)):
             course = self.store.create_course(org, course, run, self.user.id, fields=course_fields)
-                self.course_loc = course.location
+            self.course_loc = course.location  # pylint: disable=attribute-defined-outside-init
 
             def create_sub_tree(parent_loc, block_info):
+                """Recursively creates a sub_tree on this parent_loc with this block."""
                 block = self.store.create_child(
                     self.user.id,
                     # TODO remove version_agnostic() when we impl the single transaction
@@ -318,14 +320,14 @@ class ModuleStoreTestCase(TestCase):
         Create an equivalent to the toy xml course
         """
 #        with self.store.bulk_operations(self.store.make_course_key(org, course, run)):
-        self.toy_loc = self.create_sample_course(
+        self.toy_loc = self.create_sample_course(  # pylint: disable=attribute-defined-outside-init
             org, course, run, TOY_BLOCK_INFO_TREE,
             {
-                "textbooks" : [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]],
-                "wiki_slug" : "toy",
-                "display_name" : "Toy Course",
-                "graded" : True,
-                "tabs" : [
+                "textbooks": [["Textbook", "https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"]],
+                "wiki_slug": "toy",
+                "display_name": "Toy Course",
+                "graded": True,
+                "tabs": [
                     CoursewareTab(),
                     CourseInfoTab(),
                     StaticTab(name="Syllabus", url_slug="syllabus"),
@@ -334,27 +336,27 @@ class ModuleStoreTestCase(TestCase):
                     WikiTab(),
                     ProgressTab(),
                 ],
-                "discussion_topics" : {"General" : {"id" : "i4x-edX-toy-course-2012_Fall"}},
-                "graceperiod" : datetime.timedelta(days=2, seconds=21599),
-                "start" : datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc),
-                "xml_attributes" : {"filename" : ["course/2012_Fall.xml", "course/2012_Fall.xml"]},
-                "pdf_textbooks" : [
+                "discussion_topics": {"General": {"id": "i4x-edX-toy-course-2012_Fall"}},
+                "graceperiod": datetime.timedelta(days=2, seconds=21599),
+                "start": datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc),
+                "xml_attributes": {"filename": ["course/2012_Fall.xml", "course/2012_Fall.xml"]},
+                "pdf_textbooks": [
                     {
-                        "tab_title" : "Sample Multi Chapter Textbook",
-                        "id" : "MyTextbook",
-                        "chapters" : [
-                             {"url" : "/static/Chapter1.pdf", "title" : "Chapter 1"},
-                             {"url" : "/static/Chapter2.pdf", "title" : "Chapter 2"}
+                        "tab_title": "Sample Multi Chapter Textbook",
+                        "id": "MyTextbook",
+                        "chapters": [
+                            {"url": "/static/Chapter1.pdf", "title": "Chapter 1"},
+                            {"url": "/static/Chapter2.pdf", "title": "Chapter 2"}
                         ]
                     }
                 ],
-                "course_image" : "just_a_test.jpg",
+                "course_image": "just_a_test.jpg",
             }
         )
         with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.toy_loc):
             self.store.create_item(
                 self.user.id, self.toy_loc, "about", block_id="short_description",
-                fields={"data" : "A course about toys."}
+                fields={"data": "A course about toys."}
             )
             self.store.create_item(
                 self.user.id, self.toy_loc, "about", block_id="effort",

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

@@ -7,8 +7,8 @@ The data type and use of it for declaratively creating test courses.
 # fields is a dictionary of keys and values. sub_tree is a collection of BlockInfo
 from collections import namedtuple
 import datetime
-BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree')
-default_block_info_tree = [
+BlockInfo = namedtuple('BlockInfo', 'block_id, category, fields, sub_tree')  # pylint: disable=invalid-name
+default_block_info_tree = [  # pylint: disable=invalid-name
     BlockInfo(
         'chapter_x', 'chapter', {}, [
             BlockInfo(
@@ -44,7 +44,7 @@ default_block_info_tree = [
 # equivalent to toy course in xml
 TOY_BLOCK_INFO_TREE = [
     BlockInfo(
-        'Overview', "chapter", {"display_name" : "Overview"}, [
+        'Overview', "chapter", {"display_name": "Overview"}, [
             BlockInfo(
                 "Toy_Videos", "videosequence", {
                     "xml_attributes": {"filename": ["", None]}, "display_name": "Toy Videos", "format": "Lecture Sequence"
@@ -52,24 +52,24 @@ TOY_BLOCK_INFO_TREE = [
                     BlockInfo(
                         "secret:toylab", "html", {
                             "data": "<b>Lab 2A: Superposition Experiment</b>\n\n<<<<<<< Updated upstream\n<p>Isn't the toy course great?</p>\n\n<p>Let's add some markup that uses non-ascii characters.\nFor example, we should be able to write words like encyclop&aelig;dia, or foreign words like fran&ccedil;ais.\nLooking beyond latin-1, we should handle math symbols:  &pi;r&sup2 &le; &#8734.\nAnd it shouldn't matter if we use entities or numeric codes &mdash; &Omega; &ne; &pi; &equiv; &#937; &#8800; &#960;.\n</p>\n=======\n<p>Isn't the toy course great? — &le;</p>\n>>>>>>> Stashed changes\n",
-                            "xml_attributes": { "filename" : [  "html/secret/toylab.xml", "html/secret/toylab.xml" ] },
-                            "display_name" : "Toy lab"
+                            "xml_attributes": {"filename": ["html/secret/toylab.xml", "html/secret/toylab.xml"]},
+                            "display_name": "Toy lab"
                         }, []
                     ),
                     BlockInfo(
                         "toyjumpto", "html", {
-                            "data" : "<a href=\"/jump_to_id/vertical_test\">This is a link to another page and some Chinese 四節比分和七年前</a> <p>Some more Chinese 四節比分和七年前</p>\n",
-                            "xml_attributes": { "filename" : [  "html/toyjumpto.xml", "html/toyjumpto.xml" ] }
+                            "data": "<a href=\"/jump_to_id/vertical_test\">This is a link to another page and some Chinese 四節比分和七年前</a> <p>Some more Chinese 四節比分和七年前</p>\n",
+                            "xml_attributes": {"filename": ["html/toyjumpto.xml", "html/toyjumpto.xml"]}
                         }, []),
                     BlockInfo(
                         "toyhtml", "html", {
-                            "data" : "<a href='/static/handouts/sample_handout.txt'>Sample</a>",
-                            "xml_attributes" : { "filename" : [  "html/toyhtml.xml", "html/toyhtml.xml" ] }
+                            "data": "<a href='/static/handouts/sample_handout.txt'>Sample</a>",
+                            "xml_attributes": {"filename": ["html/toyhtml.xml", "html/toyhtml.xml"]}
                         }, []),
                     BlockInfo(
                         "nonportable", "html", {
                             "data": "<a href=\"/static/foo.jpg\">link</a>\n",
-                            "xml_attributes" : { "filename" : [  "html/nonportable.xml", "html/nonportable.xml" ] }
+                            "xml_attributes": {"filename": ["html/nonportable.xml", "html/nonportable.xml"]}
                         }, []),
                     BlockInfo(
                         "nonportable_link", "html", {
@@ -79,7 +79,7 @@ TOY_BLOCK_INFO_TREE = [
                     BlockInfo(
                         "badlink", "html", {
                             "data": "<img src=\"/static//file.jpg\" />\n",
-                            "xml_attributes" : { "filename" : [  "html/badlink.xml", "html/badlink.xml" ] }
+                            "xml_attributes": {"filename": ["html/badlink.xml", "html/badlink.xml"]}
                         }, []),
                     BlockInfo(
                         "with_styling", "html", {
@@ -89,11 +89,11 @@ TOY_BLOCK_INFO_TREE = [
                     BlockInfo(
                         "just_img", "html", {
                             "data": "<img src=\"/static/foo_bar.jpg\" />",
-                            "xml_attributes": {"filename": [  "html/just_img.xml", "html/just_img.xml" ] }
+                            "xml_attributes": {"filename": ["html/just_img.xml", "html/just_img.xml"]}
                         }, []),
                     BlockInfo(
                         "Video_Resources", "video", {
-                            "youtube_id_1_0" : "1bK-WdDi6Qw", "display_name" : "Video Resources"
+                            "youtube_id_1_0": "1bK-WdDi6Qw", "display_name": "Video Resources"
                         }, []),
                 ]),
             BlockInfo(
@@ -109,7 +109,7 @@ TOY_BLOCK_INFO_TREE = [
     ),
     BlockInfo(
         "secret:magic", "chapter", {
-            "xml_attributes": {"filename": [ "chapter/secret/magic.xml", "chapter/secret/magic.xml"]}
+            "xml_attributes": {"filename": ["chapter/secret/magic.xml", "chapter/secret/magic.xml"]}
         }, [
             BlockInfo(
                 "toyvideo", "video", {"youtube_id_1_0": "OEoXaMPEzfMA", "display_name": "toyvideo"}, []
@@ -124,7 +124,7 @@ TOY_BLOCK_INFO_TREE = [
                     "answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}],
                     "xml_attributes": {"reset": "false", "filename": ["", None]},
                     "display_name": "Change your answer"
-                }, []) ]
+                }, [])]
     ),
     BlockInfo(
         "vertical_container", "chapter", {
@@ -163,7 +163,7 @@ TOY_BLOCK_INFO_TREE = [
                         "T1_changemind_poll_foo_2", "poll_question", {
                             "question": "<p>Have you changed your mind?</p>",
                             "answers": [{"text": "Yes", "id": "yes"}, {"text": "No", "id": "no"}],
-                            "xml_attributes": {"reset": "false", "filename": [  "", None]},
+                            "xml_attributes": {"reset": "false", "filename": ["", None]},
                             "display_name": "Change your answer"
                         }, []),
                 ]),
@@ -175,7 +175,7 @@ TOY_BLOCK_INFO_TREE = [
     ),
     BlockInfo(
         "handout_container", "chapter", {
-            "xml_attributes" : {"filename" : ["chapter/handout_container.xml", "chapter/handout_container.xml"]}
+            "xml_attributes": {"filename": ["chapter/handout_container.xml", "chapter/handout_container.xml"]}
         }, [
             BlockInfo(
                 "html_7e5578f25f79", "html", {

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

@@ -451,7 +451,7 @@ class SplitModuleTest(unittest.TestCase):
     }
 
     @staticmethod
-    def bootstrapDB(split_store):
+    def bootstrapDB(split_store):  # pylint: disable=invalid-name
         '''
         Sets up the initial data into the db
         '''
@@ -517,7 +517,7 @@ class SplitModuleTest(unittest.TestCase):
             SplitModuleTest.modulestore = None
         super(SplitModuleTest, self).tearDown()
 
-    def findByIdInResult(self, collection, _id):
+    def findByIdInResult(self, collection, _id):  # pylint: disable=invalid-name
         """
         Result is a collection of descriptors. Find the one whose block id
         matches the _id.
@@ -716,6 +716,7 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertEqual(len(result.children[0].children), 1)
         self.assertEqual(result.children[0].children[0].locator.version_guid, versions[0])
 
+
 class SplitModuleItemTests(SplitModuleTest):
     '''
     Item read tests including inheritance
@@ -946,6 +947,10 @@ class SplitModuleItemTests(SplitModuleTest):
 
 
 def version_agnostic(children):
+    """
+    children: list of descriptors
+    Returns the `children` list with each member version-agnostic
+    """
     return [child.version_agnostic() for child in children]
 
 
@@ -1035,7 +1040,6 @@ class TestItemCrud(SplitModuleTest):
         self.assertIn(new_module.location.version_agnostic(), version_agnostic(parent.children))
         self.assertEqual(new_module.definition_locator.definition_id, original.definition_locator.definition_id)
 
-
     def test_unique_naming(self):
         """
         Check that 2 modules of same type get unique block_ids. Also check that if creation provides
@@ -1760,6 +1764,6 @@ def modulestore():
     return SplitModuleTest.modulestore
 
 
-# pylint: disable=W0613
+# pylint: disable=unused-argument, missing-docstring
 def render_to_template_mock(*args):
     pass

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -86,6 +86,7 @@ class PeerGradingFields(object):
         scope=Scope.content
     )
 
+
 class InvalidLinkLocation(Exception):
     """
     Exception for the case in which a peer grading module tries to link to an invalid location.
@@ -150,7 +151,8 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         try:
             self.student_data_for_location = json.loads(self.student_data_for_location)
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
+            # OK with this broad exception because we just want to continue on any error
             pass
 
     @property
@@ -218,9 +220,9 @@ class PeerGradingModule(PeerGradingFields, XModule):
             # This is a dev_facing_error
             return json.dumps({'error': 'Error handling action.  Please try again.', 'success': False})
 
-        d = handlers[dispatch](data)
+        data_dict = handlers[dispatch](data)
 
-        return json.dumps(d, cls=ComplexEncoder)
+        return json.dumps(data_dict, cls=ComplexEncoder)
 
     def query_data_for_location(self, location):
         student_id = self.system.anonymous_student_id
@@ -229,13 +231,12 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         try:
             response = self.peer_gs.get_data_for_location(location, student_id)
-            count_graded = response['count_graded']
-            count_required = response['count_required']
+            _count_graded = response['count_graded']
+            _count_required = response['count_required']
             success = True
         except GradingServiceError:
             # This is a dev_facing_error
-            log.exception("Error getting location data from controller for location {0}, student {1}"
-            .format(location, student_id))
+            log.exception("Error getting location data from controller for location %s, student %s", location, student_id)
 
         return success, response
 
@@ -322,8 +323,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return response
         except GradingServiceError:
             # This is a dev_facing_error
-            log.exception("Error getting next submission.  server url: {0}  location: {1}, grader_id: {2}"
-            .format(self.peer_gs.url, location, grader_id))
+            log.exception("Error getting next submission.  server url: %s  location: %s, grader_id: %s", self.peer_gs.url, location, grader_id)
             # This is a student_facing_error
             return {'success': False,
                     'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
@@ -355,7 +355,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         if not success:
             return self._err_response(message)
 
-        data_dict = {k:data.get(k) for k in required}
+        data_dict = {k: data.get(k) for k in required}
         if 'rubric_scores[]' in required:
             data_dict['rubric_scores'] = data.getall('rubric_scores[]')
         data_dict['grader_id'] = self.system.anonymous_student_id
@@ -365,15 +365,14 @@ class PeerGradingModule(PeerGradingFields, XModule):
             success, location_data = self.query_data_for_location(data_dict['location'])
             #Don't check for success above because the response = statement will raise the same Exception as the one
             #that will cause success to be false.
-            response.update({'required_done' : False})
-            if 'count_graded' in location_data and 'count_required' in location_data and int(location_data['count_graded'])>=int(location_data['count_required']):
+            response.update({'required_done': False})
+            if 'count_graded' in location_data and 'count_required' in location_data and int(location_data['count_graded']) >= int(location_data['count_required']):
                 response['required_done'] = True
             return response
         except GradingServiceError:
             # This is a dev_facing_error
-            log.exception("""Error saving grade to open ended grading service.  server url: {0}"""
-            .format(self.peer_gs.url)
-            )
+            log.exception("Error saving grade to open ended grading service.  server url: %s", self.peer_gs.url)
+
             # This is a student_facing_error
             return {
                 'success': False,
@@ -411,8 +410,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return response
         except GradingServiceError:
             # This is a dev_facing_error
-            log.exception("Error from open ended grading service.  server url: {0}, grader_id: {0}, location: {1}"
-            .format(self.peer_gs.url, grader_id, location))
+            log.exception("Error from open ended grading service.  server url: %s, grader_id: %s, location: %s", self.peer_gs.url, grader_id, location)
             # This is a student_facing_error
             return {
                 'success': False,
@@ -456,8 +454,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return response
         except GradingServiceError:
             # This is a dev_facing_error
-            log.exception("Error from open ended grading service.  server url: {0}, location: {0}"
-            .format(self.peer_gs.url, location))
+            log.exception("Error from open ended grading service.  server url: %s, location: %s", self.peer_gs.url, location)
             # This is a student_facing_error
             return {'success': False,
                     'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
@@ -492,7 +489,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         if not success:
             return self._err_response(message)
 
-        data_dict = {k:data.get(k) for k in required}
+        data_dict = {k: data.get(k) for k in required}
         data_dict['rubric_scores'] = data.getall('rubric_scores[]')
         data_dict['student_id'] = self.system.anonymous_student_id
         data_dict['calibration_essay_id'] = data_dict['submission_id']
@@ -619,7 +616,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         elif data.get('location') is not None:
             problem_location = self.course_id.make_usage_key_from_deprecated_string(data.get('location'))
 
-        module = self._find_corresponding_module_for_location(problem_location)
+        module = self._find_corresponding_module_for_location(problem_location)  # pylint: disable-unused-variable
 
         ajax_url = self.ajax_url
         html = self.system.render_template('peer_grading/peer_grading_problem.html', {

lms/djangoapps/dashboard/views.py

+"""View functions for the LMS Student dashboard"""
 from django.http import Http404
 from edxmako.shortcuts import render_to_response
 from django.db import connection
@@ -15,15 +16,18 @@ def dictfetchall(cursor):
 
     # ensure response from db is a list, not a tuple (which is returned
     # by MySQL backed django instances)
-    rows_from_cursor=cursor.fetchall()
+    rows_from_cursor = cursor.fetchall()
     table = table + [list(row) for row in rows_from_cursor]
     return table
 
-def SQL_query_to_list(cursor, query_string):
+
+def SQL_query_to_list(cursor, query_string):  # pylint: disable=invalid-name
+    """Returns the raw result of the query"""
     cursor.execute(query_string)
-    raw_result=dictfetchall(cursor)
+    raw_result = dictfetchall(cursor)
     return raw_result
 
+
 def dashboard(request):
     """
     Slightly less hackish hack to show staff enrollment numbers and other
@@ -40,11 +44,11 @@ def dashboard(request):
     # two types of results: scalars and tables.  Scalars should be represented
     # as "Visible Title": Value and tables should be lists of lists where each
     # inner list represents a single row of the table
-    results = {"scalars":{},"tables":{}}
+    results = {"scalars": {}, "tables": {}}
 
     # count how many users we have
-    results["scalars"]["Unique Usernames"]=User.objects.filter().count()
-    results["scalars"]["Activated Usernames"]=User.objects.filter(is_active=1).count()
+    results["scalars"]["Unique Usernames"] = User.objects.filter().count()
+    results["scalars"]["Activated Usernames"] = User.objects.filter(is_active=1).count()
 
     # count how many enrollments we have
     results["scalars"]["Total Enrollments Across All Courses"] = CourseEnrollment.objects.filter(is_active=1).count()
@@ -78,6 +82,6 @@ def dashboard(request):
         cursor.execute(table_queries[query])
         results["tables"][query] = SQL_query_to_list(cursor, table_queries[query])
 
-    context={"results":results}
+    context = {"results": results}
 
-    return render_to_response("admin_dashboard.html",context)
+    return render_to_response("admin_dashboard.html", context)

lms/djangoapps/django_comment_client/forum/tests.py

@@ -444,6 +444,7 @@ class UserProfileTestCase(ModuleStoreTestCase):
         )
         self.assertEqual(response.status_code, 405)
 
+
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 @patch('requests.request')
 class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):

lms/djangoapps/instructor/views/legacy.py

@@ -3,7 +3,7 @@ Instructor Views
 """
 ## NOTE: This is the code for the legacy instructor dashboard
 ## We are no longer supporting this file or accepting changes into it.
-
+# pylint: skip-file
 from contextlib import contextmanager
 import csv
 import json
@@ -1427,6 +1427,7 @@ def get_student_grade_summary_data(request, course, get_grades=True, get_raw_sco
 
 # Gradebook has moved to instructor.api.spoc_gradebook #
 
+
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 def grade_summary(request, course_key):
     """Display the grade summary for a course."""

lms/djangoapps/shoppingcart/models.py

@@ -24,7 +24,7 @@ from xmodule.modulestore.django import modulestore
 
 from course_modes.models import CourseMode
 from edxmako.shortcuts import render_to_string
-from student.models import CourseEnrollment, unenroll_done
+from student.models import CourseEnrollment, UNENROLL_DONE
 from util.query import use_read_replica_if_available
 from xmodule_django.models import CourseKeyField
 
@@ -639,7 +639,7 @@ class CertificateItem(OrderItem):
     course_enrollment = models.ForeignKey(CourseEnrollment)
     mode = models.SlugField()
 
-    @receiver(unenroll_done)
+    @receiver(UNENROLL_DONE)
     def refund_cert_callback(sender, course_enrollment=None, **kwargs):
         """
         When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment

lms/lib/comment_client/comment_client.py

-# Import other classes here so they can be imported from here.
-# pylint: disable=W0611
+"""Import other classes here so they can be imported from here."""
+# pylint: disable=unused-import
 from .comment import Comment
 from .thread import Thread
 from .user import User

lms/lib/comment_client/commentable.py

+"""Provides base Commentable model class"""
 import models
 import settings
 
-
 class Commentable(models.Model):
 
     base_url = "{prefix}/commentables".format(prefix=settings.PREFIX)