Merge pull request #7013 from stvstnfrd/pylint/low

Fix Pylint violations (easy)

cms/djangoapps/contentstore/views/component.py

@@ -331,7 +331,6 @@ def get_component_templates(courselike, library=False):
                         "Advanced component %s does not exist. It will not be added to the Studio new component menu.",
                         category
                     )
-                    pass
     else:
         log.error(
             "Improper format for course advanced keys! %s",

cms/djangoapps/contentstore/views/xblock.py

@@ -25,7 +25,7 @@ def xblock_resource(request, block_type, uri):  # pylint: disable=unused-argumen
     except IOError:
         log.info('Failed to load xblock resource', exc_info=True)
         raise Http404
-    except Exception:  # pylint: disable-msg=broad-except
+    except Exception:  # pylint: disable=broad-except
         log.error('Failed to load xblock resource', exc_info=True)
         raise Http404
 

common/lib/capa/capa/inputtypes.py

@@ -1016,8 +1016,6 @@ class Schematic(InputTypeBase):
         ]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'setup_script': '{static_url}js/capa/schematicinput.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1410,8 +1408,6 @@ class EditAMoleculeInput(InputTypeBase):
                 Attribute('missing', None)]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'applet_loader': '{static_url}js/capa/editamolecule.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1446,8 +1442,6 @@ class DesignProtein2dInput(InputTypeBase):
                 ]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'applet_loader': '{static_url}js/capa/design-protein-2d.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1482,8 +1476,6 @@ class EditAGeneInput(InputTypeBase):
                 ]
 
     def _extra_context(self):
-        """
-            """
         context = {
             'applet_loader': '{static_url}js/capa/edit-a-gene.js'.format(
                 static_url=self.capa_system.STATIC_URL),

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

@@ -27,8 +27,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
 
         # Render the HTML
         etree.XML(problem.get_html())
-        # expect that we made it here without blowing up
-        self.assertTrue(True)
+        # TODO: This test should inspect the rendered html and assert one or more things about it
 
     def test_include_html(self):
         # Create a test file to include

common/lib/capa/capa/tests/test_responsetypes.py

@@ -23,6 +23,23 @@ import calc
 from capa.responsetypes import LoncapaProblemError, \
     StudentInputError, ResponseError
 from capa.correctmap import CorrectMap
+from capa.tests.response_xml_factory import (
+    AnnotationResponseXMLFactory,
+    ChoiceResponseXMLFactory,
+    CodeResponseXMLFactory,
+    ChoiceTextResponseXMLFactory,
+    CustomResponseXMLFactory,
+    FormulaResponseXMLFactory,
+    ImageResponseXMLFactory,
+    JavascriptResponseXMLFactory,
+    MultipleChoiceResponseXMLFactory,
+    NumericalResponseXMLFactory,
+    OptionResponseXMLFactory,
+    SchematicResponseXMLFactory,
+    StringResponseXMLFactory,
+    SymbolicResponseXMLFactory,
+    TrueFalseResponseXMLFactory,
+)
 from capa.util import convert_files_to_filenames
 from capa.util import compare_with_tolerance
 from capa.xqueue_interface import dateformat
@@ -77,7 +94,6 @@ class ResponseTest(unittest.TestCase):
 
 
 class MultiChoiceResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
     xml_factory_class = MultipleChoiceResponseXMLFactory
 
     def test_multiple_choice_grade(self):
@@ -99,7 +115,6 @@ class MultiChoiceResponseTest(ResponseTest):
 
 
 class TrueFalseResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import TrueFalseResponseXMLFactory
     xml_factory_class = TrueFalseResponseXMLFactory
 
     def test_true_false_grade(self):
@@ -139,7 +154,6 @@ class TrueFalseResponseTest(ResponseTest):
 
 
 class ImageResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import ImageResponseXMLFactory
     xml_factory_class = ImageResponseXMLFactory
 
     def test_rectangle_grade(self):
@@ -203,7 +217,6 @@ class ImageResponseTest(ResponseTest):
 
 
 class SymbolicResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import SymbolicResponseXMLFactory
     xml_factory_class = SymbolicResponseXMLFactory
 
     def test_grade_single_input_correct(self):
@@ -321,7 +334,6 @@ class SymbolicResponseTest(ResponseTest):
 
 
 class OptionResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import OptionResponseXMLFactory
     xml_factory_class = OptionResponseXMLFactory
 
     def test_grade(self):
@@ -372,7 +384,6 @@ class FormulaResponseTest(ResponseTest):
     """
     Test the FormulaResponse class
     """
-    from capa.tests.response_xml_factory import FormulaResponseXMLFactory
     xml_factory_class = FormulaResponseXMLFactory
 
     def test_grade(self):
@@ -521,7 +532,6 @@ class FormulaResponseTest(ResponseTest):
 
 
 class StringResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import StringResponseXMLFactory
     xml_factory_class = StringResponseXMLFactory
 
     def test_backward_compatibility_for_multiple_answers(self):
@@ -871,7 +881,6 @@ class StringResponseTest(ResponseTest):
 
 
 class CodeResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import CodeResponseXMLFactory
     xml_factory_class = CodeResponseXMLFactory
 
     def setUp(self):
@@ -1063,7 +1072,6 @@ class CodeResponseTest(ResponseTest):
 
 
 class ChoiceResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import ChoiceResponseXMLFactory
     xml_factory_class = ChoiceResponseXMLFactory
 
     def test_radio_group_grade(self):
@@ -1106,7 +1114,6 @@ class ChoiceResponseTest(ResponseTest):
 
 
 class JavascriptResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import JavascriptResponseXMLFactory
     xml_factory_class = JavascriptResponseXMLFactory
 
     def test_grade(self):
@@ -1147,7 +1154,6 @@ class JavascriptResponseTest(ResponseTest):
 
 
 class NumericalResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import NumericalResponseXMLFactory
     xml_factory_class = NumericalResponseXMLFactory
 
     # We blend the line between integration (using evaluator) and exclusively
@@ -1372,7 +1378,6 @@ class NumericalResponseTest(ResponseTest):
 
 
 class CustomResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import CustomResponseXMLFactory
     xml_factory_class = CustomResponseXMLFactory
 
     def test_inline_code(self):
@@ -1924,7 +1929,6 @@ class SchematicResponseTest(ResponseTest):
     """
     Class containing setup and tests for Schematic responsetype.
     """
-    from capa.tests.response_xml_factory import SchematicResponseXMLFactory
     xml_factory_class = SchematicResponseXMLFactory
 
     def test_grade(self):
@@ -1975,7 +1979,6 @@ class SchematicResponseTest(ResponseTest):
 
 
 class AnnotationResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import AnnotationResponseXMLFactory
     xml_factory_class = AnnotationResponseXMLFactory
 
     def test_grade(self):
@@ -2017,7 +2020,6 @@ class ChoiceTextResponseTest(ResponseTest):
     Class containing setup and tests for ChoiceText responsetype.
     """
 
-    from response_xml_factory import ChoiceTextResponseXMLFactory
     xml_factory_class = ChoiceTextResponseXMLFactory
 
     # `TEST_INPUTS` is a dictionary mapping from
@@ -2188,13 +2190,6 @@ class ChoiceTextResponseTest(ResponseTest):
         with self.assertRaises(Exception):
             self.build_problem(type="invalidtextgroup")
 
-    def test_valid_xml(self):
-        """
-        Test that `build_problem` builds valid xml
-        """
-        self.build_problem()
-        self.assertTrue(True)
-
     def test_unchecked_input_not_validated(self):
         """
         Test that a student can have a non numeric answer in an unselected

common/lib/xmodule/xmodule/exceptions.py

@@ -53,4 +53,4 @@ class HeartbeatFailure(Exception):
         In addition to a msg, provide the name of the service.
         """
         self.service = service
-        return super(HeartbeatFailure, self).__init__(msg)
+        super(HeartbeatFailure, self).__init__(msg)

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

@@ -62,7 +62,7 @@ class TestMixedModuleStore(CourseComparisonTest):
     ASSET_COLLECTION = 'assetstore'
     FS_ROOT = DATA_DIR
     DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
-    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
     MONGO_COURSEID = 'MITx/999/2013_Spring'
     XML_COURSEID1 = 'edX/toy/2012_Fall'

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

@@ -54,7 +54,7 @@ COLLECTION = 'modulestore'
 ASSET_COLLECTION = 'assetstore'
 FS_ROOT = DATA_DIR  # TODO (vshnayder): will need a real fs_root for testing load_item
 DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
-RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
 
 class ReferenceTestXBlock(XBlock, XModuleMixin):

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

@@ -28,7 +28,7 @@ class ModuleStoreNoSettings(unittest.TestCase):
     COLLECTION = 'modulestore'
     FS_ROOT = DATA_DIR
     DEFAULT_CLASS = 'xmodule.modulestore.tests.test_xml_importer.StubXBlock'
-    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
     modulestore_options = {
         'default_class': DEFAULT_CLASS,

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -706,7 +706,7 @@ class XMLModuleStore(ModuleStoreReadBase):
         """
         return CourseLocator(org, course, run, deprecated=True)
 
-    def get_courses(self, depth=0, **kwargs):
+    def get_courses(self, **kwargs):
         """
         Returns a list of course descriptors.  If there were errors on loading,
         some of these may be ErrorDescriptors instead.

common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py

@@ -117,13 +117,12 @@ class PeerGradingService(GradingService):
         return result
 
 
-"""
-This is a mock peer grading service that can be used for unit tests
-without making actual service calls to the grading controller
-"""
-
-
 class MockPeerGradingService(object):
+    """
+    This is a mock peer grading service that can be used for unit tests
+    without making actual service calls to the grading controller
+    """
+
     def get_next_submission(self, problem_location, grader_id):
         return {
             'success': True,

common/lib/xmodule/xmodule/tabs.py

@@ -1,12 +1,6 @@
 """
 Implement CourseTab
 """
-# pylint: disable=incomplete-protocol
-# Note: pylint complains that we do not implement __delitem__ and __len__, although we implement __setitem__
-# and __getitem__.  However, the former two do not apply to the CourseTab class so we do not implement them.
-# The reason we implement the latter two is to enable callers to continue to use the CourseTab object with
-# dict-type accessors.
-
 from abc import ABCMeta, abstractmethod
 from xblock.fields import List
 
@@ -15,7 +9,7 @@ from xblock.fields import List
 _ = lambda text: text
 
 
-class CourseTab(object):  # pylint: disable=incomplete-protocol
+class CourseTab(object):
     """
     The Course Tab class is a data abstraction for all tabs (i.e., course navigation links) within a course.
     It is an abstract class - to be inherited by various tab types.

common/lib/xmodule/xmodule/tests/test_error_module.py

@@ -119,8 +119,8 @@ class TestErrorModuleConstruction(unittest.TestCase):
     """
     Test that error module construction happens correctly
     """
-
     def setUp(self):
+        # pylint: disable=abstract-class-instantiated
         super(TestErrorModuleConstruction, self).setUp()
         field_data = Mock(spec=FieldData)
         self.descriptor = BrokenDescriptor(

lms/djangoapps/bulk_email/forms.py

@@ -17,7 +17,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 log = logging.getLogger(__name__)
 
 
-class CourseEmailTemplateForm(forms.ModelForm):  # pylint: disable=incomplete-protocol
+class CourseEmailTemplateForm(forms.ModelForm):
     """Form providing validation of CourseEmail templates."""
 
     name = forms.CharField(required=False)
@@ -73,7 +73,7 @@ class CourseEmailTemplateForm(forms.ModelForm):  # pylint: disable=incomplete-pr
         return name
 
 
-class CourseAuthorizationAdminForm(forms.ModelForm):  # pylint: disable=incomplete-protocol
+class CourseAuthorizationAdminForm(forms.ModelForm):
     """Input form for email enabling, allowing us to verify data."""
 
     class Meta:  # pylint: disable=missing-docstring

lms/djangoapps/bulk_email/tasks.py

@@ -92,7 +92,7 @@ BULK_EMAIL_FAILURE_ERRORS = (
 )
 
 
-def _get_recipient_queryset(user_id, to_option, course_id, course_location):
+def _get_recipient_queryset(user_id, to_option, course_id):
     """
     Returns a query set of email recipients corresponding to the requested to_option category.
 
@@ -230,7 +230,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name)
         )
         return new_subtask
 
-    recipient_qset = _get_recipient_queryset(user_id, to_option, course_id, course.location)
+    recipient_qset = _get_recipient_queryset(user_id, to_option, course_id)
     recipient_fields = ['profile__name', 'email']
 
     log.info(u"Task %s: Preparing to queue subtasks for sending emails for course %s, email %s, to_option %s",

lms/djangoapps/certificates/models.py

@@ -1,13 +1,3 @@
-from django.contrib.auth.models import User
-from django.db import models
-from django.db.models.signals import post_save
-from django.dispatch import receiver
-from django.conf import settings
-from datetime import datetime
-from model_utils import Choices
-from xmodule_django.models import CourseKeyField, NoneToEmptyManager
-from util.milestones_helpers import fulfill_course_milestone
-
 """
 Certificates are created for a student and an offering of a course.
 
@@ -55,6 +45,16 @@ Eligibility:
        unless he has allow_certificate set to False.
 """
 
+from django.contrib.auth.models import User
+from django.db import models
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+from django.conf import settings
+from datetime import datetime
+from model_utils import Choices
+from xmodule_django.models import CourseKeyField, NoneToEmptyManager
+from util.milestones_helpers import fulfill_course_milestone
+
 
 class CertificateStatuses(object):
     deleted = 'deleted'

lms/djangoapps/circuit/views.py

@@ -31,7 +31,7 @@ def circuit_line(circuit):
     return xml.etree.ElementTree.tostring(circuit_line)
 
 
-def edit_circuit(request, circuit):
+def edit_circuit(_request, circuit):
     try:
         sc = ServerCircuit.objects.get(name=circuit)
     except:

lms/djangoapps/courseware/access.py

@@ -96,7 +96,7 @@ def has_access(user, action, obj, course_key=None):
         return _has_access_location(user, action, obj, course_key)
 
     if isinstance(obj, basestring):
-        return _has_access_string(user, action, obj, course_key)
+        return _has_access_string(user, action, obj)
 
     # Passing an unknown object here is a coding error, so rather than
     # returning a default, complain.
@@ -487,7 +487,7 @@ def _has_access_course_key(user, action, course_key):
     return _dispatch(checkers, action, user, course_key)
 
 
-def _has_access_string(user, action, perm, course_key):
+def _has_access_string(user, action, perm):
     """
     Check if user has certain special access, specified as string.  Valid strings:
 

lms/djangoapps/courseware/management/commands/clean_xml.py

@@ -45,7 +45,7 @@ def export(course, export_dir):
     return False
 
 
-def import_with_checks(course_dir, verbose=True):
+def import_with_checks(course_dir):
     all_ok = True
 
     print "Attempting to load '{0}'".format(course_dir)

lms/djangoapps/courseware/middleware.py

@@ -13,7 +13,7 @@ class RedirectUnenrolledMiddleware(object):
     Catch UserNotEnrolled errors thrown by `get_course_with_access` and redirect
     users to the course about page
     """
-    def process_exception(self, request, exception):
+    def process_exception(self, _request, exception):
         if isinstance(exception, UserNotEnrolled):
             course_key = exception.course_key
             return redirect(

lms/djangoapps/courseware/module_render.py

@@ -823,7 +823,7 @@ def xblock_resource(request, block_type, uri):  # pylint: disable=unused-argumen
     except IOError:
         log.info('Failed to load xblock resource', exc_info=True)
         raise Http404
-    except Exception:  # pylint: disable-msg=broad-except
+    except Exception:  # pylint: disable=broad-except
         log.error('Failed to load xblock resource', exc_info=True)
         raise Http404
     mimetype, _ = mimetypes.guess_type(uri)

lms/djangoapps/courseware/tests/test_access.py

@@ -88,12 +88,12 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
 
     def test__has_access_string(self):
         user = Mock(is_staff=True)
-        self.assertFalse(access._has_access_string(user, 'staff', 'not_global', self.course.course_key))
+        self.assertFalse(access._has_access_string(user, 'staff', 'not_global'))
 
         user._has_global_staff_access.return_value = True
-        self.assertTrue(access._has_access_string(user, 'staff', 'global', self.course.course_key))
+        self.assertTrue(access._has_access_string(user, 'staff', 'global'))
 
-        self.assertRaises(ValueError, access._has_access_string, user, 'not_staff', 'global', self.course.course_key)
+        self.assertRaises(ValueError, access._has_access_string, user, 'not_staff', 'global')
 
     def test__has_access_error_desc(self):
         descriptor = Mock()

lms/djangoapps/courseware/views.py

@@ -584,7 +584,7 @@ def jump_to_id(request, course_id, module_id):
 
 
 @ensure_csrf_cookie
-def jump_to(request, course_id, location):
+def jump_to(_request, course_id, location):
     """
     Show the page that contains a specific location.
 

lms/djangoapps/dashboard/models.py

@@ -6,8 +6,6 @@ from xmodule.modulestore.mongoengine_fields import CourseKeyField
 
 class CourseImportLog(mongoengine.Document):
     """Mongoengine model for git log"""
-    # pylint: disable=incomplete-protocol
-
     course_id = CourseKeyField(max_length=128)
     # NOTE: this location is not a Location object but a pathname
     location = mongoengine.StringField(max_length=168)

lms/djangoapps/dashboard/support.py

@@ -18,7 +18,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 log = logging.getLogger(__name__)
 
 
-class RefundForm(forms.Form):  # pylint: disable=incomplete-protocol
+class RefundForm(forms.Form):
     """
     Form for manual refunds
     """

lms/djangoapps/instructor_task/subtasks.py

@@ -165,7 +165,7 @@ class SubtaskStatus(object):
         self.state = state if state is not None else QUEUING
 
     @classmethod
-    def from_dict(self, d):
+    def from_dict(cls, d):
         """Construct a SubtaskStatus object from a dict representation."""
         options = dict(d)
         task_id = options['task_id']
@@ -173,9 +173,9 @@ class SubtaskStatus(object):
         return SubtaskStatus.create(task_id, **options)
 
     @classmethod
-    def create(self, task_id, **options):
+    def create(cls, task_id, **options):
         """Construct a SubtaskStatus object."""
-        return self(task_id, **options)
+        return cls(task_id, **options)
 
     def to_dict(self):
         """

lms/djangoapps/licenses/management/commands/generate_serial_numbers.py

@@ -24,8 +24,6 @@ class Command(BaseCommand):
     args = "course_id software_id count"
 
     def handle(self, *args, **options):
-        """
-        """
         course_id, software_name, count = self._parse_arguments(args)
 
         software, _ = CourseSoftware.objects.get_or_create(course_id=course_id,

lms/djangoapps/licenses/management/commands/import_serial_numbers.py

@@ -24,8 +24,6 @@ class Command(BaseCommand):
     args = "course_id software_id serial_file"
 
     def handle(self, *args, **options):
-        """
-        """
         course_id, software_name, filename = self._parse_arguments(args)
 
         software, _ = CourseSoftware.objects.get_or_create(course_id=course_id,

lms/djangoapps/notes/api.py

@@ -148,7 +148,7 @@ def create(request, course_key):
     return ApiResponse(http_response=response, data=None)
 
 
-def read(request, course_key, note_id):  # pylint: disable=unused-argument (course_key)
+def read(request, _course_key, note_id):
     '''
     Returns a single annotation object.
     '''
@@ -163,7 +163,7 @@ def read(request, course_key, note_id):  # pylint: disable=unused-argument (cour
     return ApiResponse(http_response=HttpResponse(), data=note.as_dict())
 
 
-def update(request, course_key, note_id):  # pylint: disable=unused-argument (course_key)
+def update(request, course_key, note_id):  # pylint: disable=unused-argument
     '''
     Updates an annotation object and returns a 303 with the read location.
     '''
@@ -247,7 +247,7 @@ def search(request, course_key):
     return ApiResponse(http_response=HttpResponse(), data=result)
 
 
-def root(request, course_key):  # pylint: disable=unused-argument (course_key, request)
+def root(request, course_key):  # pylint: disable=unused-argument
     '''
     Returns version information about the API.
     '''

lms/djangoapps/shoppingcart/models.py

@@ -1930,7 +1930,7 @@ class Donation(OrderItem):
         ).format(platform_name=settings.PLATFORM_NAME)
 
     @classmethod
-    def _line_item_description(self, course_id=None):
+    def _line_item_description(cls, course_id=None):
         """Create a line-item description for the donation.
 
         Includes the course display name if provided.

lms/djangoapps/survey/admin.py

@@ -7,7 +7,7 @@ from django.contrib import admin
 from survey.models import SurveyForm
 
 
-class SurveyFormAdminForm(forms.ModelForm):  # pylint: disable=incomplete-protocol
+class SurveyFormAdminForm(forms.ModelForm):
     """Form providing validation of SurveyForm content."""
 
     class Meta:  # pylint: disable=missing-docstring

lms/envs/test.py

@@ -377,7 +377,7 @@ FEATURES['CLASS_DASHBOARD'] = True
 #   Generated checkid_setup request to http://testserver/openid/provider/login/ with assocication {HMAC-SHA1}{51d49995}{s/kRmA==}
 
 import openid.oidutil
-openid.oidutil.log = lambda message, level = 0: None
+openid.oidutil.log = lambda message, level=0: None
 
 PLATFORM_NAME = "edX"
 SITE_NAME = "edx.org"

scripts/all-tests.sh

@@ -61,7 +61,7 @@ git clean -qxfd
 source scripts/jenkins-common.sh
 
 # Violations thresholds for failing the build
-PYLINT_THRESHOLD=6300
+PYLINT_THRESHOLD=6100
 PEP8_THRESHOLD=0
 
 # If the environment variable 'SHARD' is not set, default to 'all'.