fix pylint violations

lms/djangoapps/analytics/distributions.py

@@ -56,6 +56,11 @@ class ProfileDistribution(object):
         self.feature = feature
         self.feature_display_name = DISPLAY_NAMES.get(feature, feature)
 
+        # to be set later
+        self.type = None
+        self.data = None
+        self.choices_display_names = None
+
     def validate(self):
         """
         Validate this profile distribution.
@@ -63,6 +68,7 @@ class ProfileDistribution(object):
         Throws ProfileDistribution.ValidationError
         """
         def validation_assert(predicate):
+            """ Throw a ValidationError if false. """
             if not predicate:
                 raise ProfileDistribution.ValidationError()
 

lms/djangoapps/analytics/tests/test_distributions.py

@@ -45,7 +45,8 @@ class TestAnalyticsDistributions(TestCase):
         distribution = profile_distribution(self.course_id, feature)
         print distribution
         self.assertEqual(distribution.type, 'OPEN_CHOICE')
-        self.assertFalse(hasattr(distribution, 'choices_display_names'))
+        self.assertTrue(hasattr(distribution, 'choices_display_names'))
+        self.assertEqual(distribution.choices_display_names, None)
         self.assertNotIn('no_data', distribution.data)
         self.assertEqual(distribution.data[1930], 1)
 
@@ -76,6 +77,7 @@ class TestAnalyticsDistributionsNoData(TestCase):
         print distribution
         self.assertEqual(distribution.type, 'EASY_CHOICE')
         self.assertTrue(hasattr(distribution, 'choices_display_names'))
+        self.assertNotEqual(distribution.choices_display_names, None)
         self.assertIn('no_data', distribution.data)
         self.assertEqual(distribution.data['no_data'], len(self.nodata_users))
 
@@ -85,6 +87,7 @@ class TestAnalyticsDistributionsNoData(TestCase):
         distribution = profile_distribution(self.course_id, feature)
         print distribution
         self.assertEqual(distribution.type, 'OPEN_CHOICE')
-        self.assertFalse(hasattr(distribution, 'choices_display_names'))
+        self.assertTrue(hasattr(distribution, 'choices_display_names'))
+        self.assertEqual(distribution.choices_display_names, None)
         self.assertIn('no_data', distribution.data)
         self.assertEqual(distribution.data['no_data'], len(self.nodata_users))

lms/djangoapps/instructor/access.py

@@ -3,7 +3,7 @@ Access control operations for use by instructor APIs.
 
 Does not include any access control, be sure to check access before calling.
 
-TODO sync instructor and staff flags
+TO DO sync instructor and staff flags
     e.g. should these be possible?
         {instructor: true, staff: false}
         {instructor: true, staff: true}

lms/djangoapps/instructor/tests/test_access.py

@@ -11,8 +11,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from django.test.utils import override_settings
 from courseware.tests.modulestore_config import TEST_DATA_MONGO_MODULESTORE
 
-from courseware.access import (get_access_group_name,
-                               course_beta_test_group_name)
+from courseware.access import get_access_group_name
 from django_comment_common.models import (Role,
                                           FORUM_ROLE_MODERATOR)
 from instructor.access import (allow_access,

lms/djangoapps/instructor/tests/test_api.py

@@ -103,9 +103,10 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.notregistered_email = 'robot-not-an-email-yet@robot.org'
         self.assertEqual(User.objects.filter(email=self.notregistered_email).count(), 0)
 
-        # enable printing of large diffs
+        # uncomment to enable enable printing of large diffs
         # from failed assertions in the event of a test failure.
-        self.maxDiff = None
+        # (comment because pylint C0103)
+        # self.maxDiff = None
 
     def test_missing_params(self):
         """ Test missing all query parameters. """
@@ -605,11 +606,12 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """ Fake task object """
         FEATURES = ['task_type', 'task_input', 'task_id', 'requester', 'created', 'task_state']
 
-        def __init__(self, i):
+        def __init__(self):
             for feature in self.FEATURES:
                 setattr(self, feature, 'expected')
 
         def to_dict(self):
+            """ Convert fake task to dictionary representation. """
             return {key: 'expected' for key in self.FEATURES}
 
     def setUp(self):
@@ -631,7 +633,7 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
             state=json.dumps({'attempts': 10}),
         )
 
-        self.tasks = [self.FakeTask(i) for i in xrange(6)]
+        self.tasks = [self.FakeTask() for _ in xrange(6)]
 
     def test_list_instructor_tasks_running(self):
         """ Test list of all running tasks. """

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -30,8 +30,9 @@ class TestSettableEnrollmentState(TestCase):
             allowed=False,
             auto_enroll=False
         )
-        email, user, cenr, cea = mes.create_user(self.course_id)
-        ees = EmailEnrollmentState(self.course_id, email)
+        # enrollment objects
+        eobjs = mes.create_user(self.course_id)
+        ees = EmailEnrollmentState(self.course_id, eobjs.email)
         self.assertEqual(mes, ees)
 
 
@@ -58,23 +59,20 @@ class TestEnrollmentChangeBase(TestCase):
             `action` should transition the world from before_ideal to after_ideal
             `action` will be supplied the following arguments (None-able arguments)
                 `email` is an email string
-                `user` is a User
-                `cenr` is a CourseEnrollment
-                `cea` is a CourseEnrollmentAllowed
         """
         # initialize & check before
         print "checking initialization..."
-        email, user, cenr, cea = before_ideal.create_user(self.course_id)
-        before = EmailEnrollmentState(self.course_id, email)
+        eobjs = before_ideal.create_user(self.course_id)
+        before = EmailEnrollmentState(self.course_id, eobjs.email)
         self.assertEqual(before, before_ideal)
 
         # do action
         print "running action..."
-        action(email, user, cenr, cea)
+        action(eobjs.email)
 
         # check after
         print "checking effects..."
-        after = EmailEnrollmentState(self.course_id, email)
+        after = EmailEnrollmentState(self.course_id, eobjs.email)
         self.assertEqual(after, after_ideal)
 
 
@@ -95,8 +93,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=False
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email)
+        action = lambda email: enroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -115,8 +112,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=False,
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email)
+        action = lambda email: enroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -135,8 +131,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=False,
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email)
+        action = lambda email: enroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -155,8 +150,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=False,
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email)
+        action = lambda email: enroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -175,8 +169,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=True,
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email, auto_enroll=True)
+        action = lambda email: enroll_email(self.course_id, email, auto_enroll=True)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -195,8 +188,7 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
             auto_enroll=False,
         )
 
-        def action(email, user, cenr, cea):
-            enroll_email(self.course_id, email, auto_enroll=False)
+        action = lambda email: enroll_email(self.course_id, email, auto_enroll=False)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -218,8 +210,7 @@ class TestInstructorUnenrollDB(TestEnrollmentChangeBase):
             auto_enroll=False
         )
 
-        def action(email, user, cenr, cea):
-            unenroll_email(self.course_id, email)
+        action = lambda email: unenroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -238,8 +229,7 @@ class TestInstructorUnenrollDB(TestEnrollmentChangeBase):
             auto_enroll=False
         )
 
-        def action(email, user, cenr, cea):
-            unenroll_email(self.course_id, email)
+        action = lambda email: unenroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -258,8 +248,7 @@ class TestInstructorUnenrollDB(TestEnrollmentChangeBase):
             auto_enroll=False
         )
 
-        def action(email, user, cenr, cea):
-            unenroll_email(self.course_id, email)
+        action = lambda email: unenroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -278,8 +267,7 @@ class TestInstructorUnenrollDB(TestEnrollmentChangeBase):
             auto_enroll=False
         )
 
-        def action(email, user, cenr, cea):
-            unenroll_email(self.course_id, email)
+        action = lambda email: unenroll_email(self.course_id, email)
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
@@ -310,6 +298,24 @@ class TestInstructorEnrollmentStudentModule(TestCase):
         self.assertEqual(StudentModule.objects.filter(student=user, course_id=self.course_id, module_state_key=msk).count(), 0)
 
 
+class EnrollmentObjects(object):
+    """
+    Container for enrollment objects.
+
+    `email` - student email
+    `user` - student User object
+    `cenr` - CourseEnrollment object
+    `cea` - CourseEnrollmentAllowed object
+
+    Any of the objects except email can be None.
+    """
+    def __init__(self, email, user, cenr, cea):
+        self.email = email
+        self.user = user
+        self.cenr = cenr
+        self.cea = cea
+
+
 class SettableEnrollmentState(EmailEnrollmentState):
     """
     Settable enrollment state.
@@ -318,7 +324,7 @@ class SettableEnrollmentState(EmailEnrollmentState):
         a call to create_user will make objects which
         correspond to the state represented in the SettableEnrollmentState.
     """
-    def __init__(self, user=False, enrollment=False, allowed=False, auto_enroll=False):
+    def __init__(self, user=False, enrollment=False, allowed=False, auto_enroll=False):  # pylint: disable=W0231
         self.user = user
         self.enrollment = enrollment
         self.allowed = allowed
@@ -351,15 +357,15 @@ class SettableEnrollmentState(EmailEnrollmentState):
                     user=user,
                     course_id=course_id
                 )
-                return (email, user, cenr, None)
+                return EnrollmentObjects(email, user, cenr, None)
             else:
-                return (email, user, None, None)
+                return EnrollmentObjects(email, user, None, None)
         elif self.allowed:
             cea = CourseEnrollmentAllowed.objects.create(
                 email=email,
                 course_id=course_id,
                 auto_enroll=self.auto_enroll,
             )
-            return (email, None, None, cea)
+            return EnrollmentObjects(email, None, None, cea)
         else:
-            return (email, None, None, None)
+            return EnrollmentObjects(email, None, None, None)

lms/djangoapps/instructor/tests/test_legacy_download_csv.py

@@ -44,9 +44,10 @@ class TestInstructorDashboardGradeDownloadCSV(LoginEnrollmentTestCase):
         self.activate_user(self.instructor)
 
         def make_instructor(course):
+            """ Create an instructor for the course. """
             group_name = _course_staff_group_name(course.location)
-            g = Group.objects.create(name=group_name)
-            g.user_set.add(User.objects.get(email=self.instructor))
+            group = Group.objects.create(name=group_name)
+            group.user_set.add(User.objects.get(email=self.instructor))
 
         make_instructor(self.toy)
 

lms/djangoapps/instructor/views/api.py

@@ -3,7 +3,7 @@ Instructor Dashboard API views
 
 JSON views which the instructor dashboard requests.
 
-TODO a lot of these GETs should be PUTs
+Many of these GETs may become PUTs in the future.
 """
 
 import re
@@ -41,7 +41,7 @@ def common_exceptions_400(func):
     Catches common exceptions and renders matching 400 errors.
     (decorator without arguments)
     """
-    def wrapped(*args, **kwargs):
+    def wrapped(*args, **kwargs):  # pylint: disable=C0111
         try:
             return func(*args, **kwargs)
         except User.DoesNotExist:
@@ -65,8 +65,8 @@ def require_query_params(*args, **kwargs):
     required_params += [(key, kwargs[key]) for key in kwargs]
     # required_params = e.g. [('action', 'enroll or unenroll'), ['emails', None]]
 
-    def decorator(func):
-        def wrapped(*args, **kwargs):
+    def decorator(func):  # pylint: disable=C0111
+        def wrapped(*args, **kwargs):  # pylint: disable=C0111
             request = args[0]
 
             error_response_data = {
@@ -108,8 +108,8 @@ def require_level(level):
     if level not in ['instructor', 'staff']:
         raise ValueError("unrecognized level '{}'".format(level))
 
-    def decorator(func):
-        def wrapped(*args, **kwargs):
+    def decorator(func):  # pylint: disable=C0111
+        def wrapped(*args, **kwargs):  # pylint: disable=C0111
             request = args[0]
             course = get_course_by_id(kwargs['course_id'])
 
@@ -339,14 +339,14 @@ def get_grading_config(request, course_id):
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @require_level('staff')
-def get_students_features(request, course_id, csv=False):
+def get_students_features(request, course_id, csv=False):  # pylint: disable=W0613
     """
     Respond with json which contains a summary of all enrolled students profile information.
 
     Responds with JSON
         {"students": [{-student-info-}, ...]}
 
-    TODO accept requests for different attribute sets.
+    TO DO accept requests for different attribute sets.
     """
     available_features = analytics.basic.AVAILABLE_FEATURES
     query_features = ['username', 'name', 'email', 'language', 'location', 'year_of_birth', 'gender',
@@ -382,8 +382,6 @@ def get_distribution(request, course_id):
     If no `feature` is supplied, will return response with an
         empty response['feature_results'] object.
     A list of available will be available in the response['available_features']
-
-    TODO respond to csv requests as well
     """
     feature = request.GET.get('feature')
     # alternate notations of None
@@ -392,9 +390,9 @@ def get_distribution(request, course_id):
     else:
         feature = str(feature)
 
-    AVAILABLE_FEATURES = analytics.distributions.AVAILABLE_PROFILE_FEATURES
+    available_features = analytics.distributions.AVAILABLE_PROFILE_FEATURES
     # allow None so that requests for no feature can list available features
-    if not feature in AVAILABLE_FEATURES + (None,):
+    if not feature in available_features + (None,):
         return HttpResponseBadRequest(
             "feature '{}' not available.".format(feature)
         )
@@ -402,7 +400,7 @@ def get_distribution(request, course_id):
     response_payload = {
         'course_id': course_id,
         'queried_feature': feature,
-        'available_features': AVAILABLE_FEATURES,
+        'available_features': available_features,
         'feature_display_names': analytics.distributions.DISPLAY_NAMES,
     }
 
@@ -518,7 +516,7 @@ def reset_student_attempts(request, course_id):
         except StudentModule.DoesNotExist:
             return HttpResponseBadRequest("Module does not exist.")
     elif all_students:
-        task = instructor_task.api.submit_reset_problem_attempts_for_all_students(request, course_id, module_state_key)
+        instructor_task.api.submit_reset_problem_attempts_for_all_students(request, course_id, module_state_key)
         response_payload['task'] = 'created'
     else:
         return HttpResponseBadRequest()
@@ -566,10 +564,10 @@ def rescore_problem(request, course_id):
     if student_email:
         response_payload['student_email'] = student_email
         student = User.objects.get(email=student_email)
-        task = instructor_task.api.submit_rescore_problem_for_student(request, course_id, module_state_key, student)
+        instructor_task.api.submit_rescore_problem_for_student(request, course_id, module_state_key, student)
         response_payload['task'] = 'created'
     elif all_students:
-        task = instructor_task.api.submit_rescore_problem_for_all_students(request, course_id, module_state_key)
+        instructor_task.api.submit_rescore_problem_for_all_students(request, course_id, module_state_key)
         response_payload['task'] = 'created'
     else:
         return HttpResponseBadRequest()
@@ -614,8 +612,8 @@ def list_instructor_tasks(request, course_id):
 
     def extract_task_features(task):
         """ Convert task to dict for json rendering """
-        FEATURES = ['task_type', 'task_input', 'task_id', 'requester', 'created', 'task_state']
-        return dict((feature, str(getattr(task, feature))) for feature in FEATURES)
+        features = ['task_type', 'task_input', 'task_id', 'requester', 'created', 'task_state']
+        return dict((feature, str(getattr(task, feature))) for feature in features)
 
     response_payload = {
         'tasks': map(extract_task_features, tasks),

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -12,10 +12,7 @@ from django.http import Http404
 from courseware.access import has_access
 from courseware.courses import get_course_by_id
 from django_comment_client.utils import has_forum_access
-from django_comment_common.models import (Role,
-                                          FORUM_ROLE_ADMINISTRATOR,
-                                          FORUM_ROLE_MODERATOR,
-                                          FORUM_ROLE_COMMUNITY_TA)
+from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
 from xmodule.modulestore.django import modulestore
 from student.models import CourseEnrollment
 
@@ -64,9 +61,9 @@ The dictionary must include at least {
     'section_display_name': 'Circus Expo'
 }
 
+section_key will be used as a css attribute, javascript tie-in, and template import filename.
 section_display_name will be used to generate link titles in the nav bar.
-sek will be used as a css attribute, javascript tie-in, and template import filename.
-"""
+"""  # pylint: disable=W0105
 
 
 def _section_course_info(course_id):
@@ -81,16 +78,16 @@ def _section_course_info(course_id):
     section_data['enrollment_count'] = CourseEnrollment.objects.filter(course_id=course_id).count()
     section_data['has_started'] = course.has_started()
     section_data['has_ended'] = course.has_ended()
+
     try:
-        def next(memo, (letter, score)):
-            return "{}: {}, ".format(letter, score) + memo
-        section_data['grade_cutoffs'] = reduce(next, course.grade_cutoffs.items(), "")[:-2]
-    except:
+        advance = lambda memo, (letter, score): "{}: {}, ".format(letter, score) + memo
+        section_data['grade_cutoffs'] = reduce(advance, course.grade_cutoffs.items(), "")[:-2]
+    except Exception:
         section_data['grade_cutoffs'] = "Not Available"
     # section_data['offline_grades'] = offline_grades_available(course_id)
 
     try:
-        section_data['course_errors'] = [(escape(a), '') for (a, b) in modulestore().get_item_errors(course.location)]
+        section_data['course_errors'] = [(escape(a), '') for (a, _) in modulestore().get_item_errors(course.location)]
     except Exception:
         section_data['course_errors'] = [('Error fetching errors', '')]