PEP8 Test and fixes, add three pylint checks as well

.travis.yml

@@ -9,6 +9,6 @@ install:
     - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/requirements.txt"
     - "pip install -r $VIRTUAL_ENV/src/xblock-sdk/test-requirements.txt"
     - "pip install -r requirements.txt"
-script: python run_tests.py --with-coverage --cover-package=mentoring
+script: python run_tests.py --with-coverage --cover-package=mentoring && pep8 mentoring --max-line-length=120 && pylint mentoring --disable=all --enable=function-redefined,undefined-variable,unused-variable
 notifications:
   email: false

mentoring/components/answer.py

@@ -83,7 +83,7 @@ class AnswerBlock(XBlock, StepMixin):
         scope=Scope.user_state,
         default=""
     )
-    # Shared student input - share answers among all Answer blocks in the course with the same name 
+    # Shared student input - share answers among all Answer blocks in the course with the same name
     student_input_shared = Dict(
         scope=Scope.preferences,
     )

mentoring/components/html.py

@@ -31,6 +31,7 @@ from xblock.fragment import Fragment
 
 # Classes ###########################################################
 
+
 class HTMLBlock(XBlock):
     """
     Render content as HTML

mentoring/components/message.py

@@ -23,14 +23,12 @@
 
 # Imports ###########################################################
 
-
-
 from .common import BlockWithContent
 from xblock.fields import Scope, String
 
-
 # Classes ###########################################################
 
+
 class MentoringMessageBlock(BlockWithContent):
     """
     A message which can be conditionally displayed at the mentoring block level,

mentoring/components/mrq.py

@@ -71,8 +71,9 @@ class MRQBlock(QuestionnaireAbstractBlock):
             }
             # Only include tips/results in returned response if we want to display them
             if not self.hide_results:
+                loader = ResourceLoader(__name__)
                 choice_result['completed'] = choice_completed
-                choice_result['tips'] = ResourceLoader(__name__).render_template('templates/html/tip_choice_group.html', {
+                choice_result['tips'] = loader.render_template('templates/html/tip_choice_group.html', {
                     'self': self,
                     'tips_html': choice_tips_html,
                     'completed': choice_completed,

mentoring/components/step.py

@@ -18,6 +18,7 @@
 # "AGPLv3".  If not, see <http://www.gnu.org/licenses/>.
 #
 
+
 class StepParentMixin(object):
     """
     An XBlock mixin for a parent block containing Step children
@@ -26,7 +27,7 @@ class StepParentMixin(object):
     @property
     def steps(self):
         """
-        Generator returning the usage_id for all of this XBlock's 
+        Generator returning the usage_id for all of this XBlock's
         children that are "Steps"
         """
         step_ids = []

mentoring/components/tip.py

@@ -28,6 +28,7 @@ from xblock.fields import Scope, String
 
 # Functions #########################################################
 
+
 def commas_to_set(commas_str):
     """
     Converts a comma-separated string to a set

mentoring/dataexport.py

@@ -24,8 +24,10 @@
 # Imports ###########################################################
 
 import logging
+import unicodecsv
 
 from itertools import groupby
+from StringIO import StringIO
 from webob import Response
 from xblock.core import XBlock
 from xblock.fields import String, Scope
@@ -33,13 +35,13 @@ from xblock.fragment import Fragment
 from xblockutils.resources import ResourceLoader
 
 
-
 # Globals ###########################################################
 
 log = logging.getLogger(__name__)
 
 # Utils ###########################################################
 
+
 def list2csv(row):
     """
     Convert a list to a CSV string (single row)
@@ -84,16 +86,18 @@ class MentoringDataExportBlock(XBlock):
         return response
 
     def get_csv(self):
-        course_id = self.xmodule_runtime.course_id
+        # course_id = self.xmodule_runtime.course_id
 
-        answers = Answer.objects.filter(course_id=course_id).order_by('student_id', 'name')
+        # TODO: Fix this method - not working yet with rewrite away from LightChildren.
+        raise NotImplementedError
+        answers = []  # Answer.objects.filter(course_id=course_id).order_by('student_id', 'name')
         answers_names = answers.values_list('name', flat=True).distinct().order_by('name')
 
         # Header line
         yield list2csv([u'student_id'] + list(answers_names))
 
         if answers_names:
-            for k, student_answers in groupby(answers, lambda x: x.student_id):
+            for _, student_answers in groupby(answers, lambda x: x.student_id):
                 row = []
                 next_answer_idx = 0
                 for answer in student_answers:

mentoring/mentoring.py

@@ -86,8 +86,11 @@ class MentoringBlock(XBlock, StepParentMixin):
     )
     url_name = String(
         help="Name of the current step, used for URL building",
-        default='mentoring-default',  # TODO in future: set this to xblock.fields.UNIQUE_ID and remove self.url_name_with_default
+        default='mentoring-default',
         scope=Scope.content
+        # TODO in future: set this field's default to xblock.fields.UNIQUE_ID
+        # and remove self.url_name_with_default. Waiting until UNIQUE_ID support
+        # is available in edx-platform's pinned version of xblock. (See XBlock PR 249)
     )
     enforce_dependency = Boolean(
         help="Should the next step be the current block to complete?",
@@ -175,7 +178,6 @@ class MentoringBlock(XBlock, StepParentMixin):
         # Migrate stored data if necessary
         self.migrate_fields()
 
-
         fragment = Fragment()
         title = u""
         header = u""
@@ -258,7 +260,7 @@ class MentoringBlock(XBlock, StepParentMixin):
         Returns True if the student needs to complete another step before being able to complete
         the current one, and False otherwise
         """
-        return self.enforce_dependency and (not self.completed) and (self.next_step != self.url_name_with_default)  # TODO: Fix
+        return self.enforce_dependency and (not self.completed) and (self.next_step != self.url_name_with_default)
 
     @property
     def next_step_url(self):

mentoring/tests/integration/test_assessment.py

@@ -2,6 +2,7 @@ from .base_test import MentoringBaseTest
 
 CORRECT, INCORRECT, PARTIAL = "correct", "incorrect", "partially-correct"
 
+
 class MentoringAssessmentTest(MentoringBaseTest):
     def _selenium_bug_workaround_scroll_to(self, mentoring, question):
         """Workaround for selenium bug:
@@ -20,7 +21,6 @@ class MentoringAssessmentTest(MentoringBaseTest):
         hopefully, this gives us enough room for the full step with the
         control buttons to fit.
         """
-        self.browser.execute_script("$('header.banner').remove();")  # Hide the Workbench header which can obscure other elements we need
         controls = mentoring.find_element_by_css_selector("div.submit")
         title = question.find_element_by_css_selector("h3.question-title")
         controls.click()
@@ -41,10 +41,6 @@ class MentoringAssessmentTest(MentoringBaseTest):
         self.assertIn("A Simple Assessment", mentoring.text)
         self.assertIn("This paragraph is shared between all questions.", mentoring.text)
 
-    def assert_disabled(self, elem):
-        self.assertTrue(elem.is_displayed())
-        self.assertFalse(elem.is_enabled())
-
     class _GetChoices(object):
         def __init__(self, question, selector=".choices"):
             self._mcq = question.find_element_by_css_selector(selector)
@@ -60,7 +56,6 @@ class MentoringAssessmentTest(MentoringBaseTest):
                 for choice in self._mcq.find_elements_by_css_selector(".choice")}
 
         def select(self, text):
-            state = {}
             for choice in self._mcq.find_elements_by_css_selector(".choice"):
                 if choice.text == text:
                     choice.find_element_by_css_selector("input").click()
@@ -75,7 +70,6 @@ class MentoringAssessmentTest(MentoringBaseTest):
         for name, count in states.items():
             self.assertEqual(len(mentoring.find_elements_by_css_selector(".checkmark-{}".format(name))), count)
 
-
     def go_to_workbench_main_page(self):
         self.browser.get(self.live_server_url)
 
@@ -302,24 +296,27 @@ class MentoringAssessmentTest(MentoringBaseTest):
         self.multiple_choice_question(4, mentoring, controls, ("Its beauty",), PARTIAL, last=True)
 
         expected_results = {
-                "correct": 2, "partial": 1, "incorrect": 1, "percentage": 63,
-                "num_attempts": 1, "max_attempts": 2}
+            "correct": 2, "partial": 1, "incorrect": 1, "percentage": 63,
+            "num_attempts": 1, "max_attempts": 2
+        }
         self.peek_at_review(mentoring, controls, expected_results)
 
         self.assert_clickable(controls.try_again)
         controls.try_again.click()
 
-        self.freeform_answer(1, mentoring, controls, 'This is a different answer', CORRECT,
-                saved_value='This is the answer')
+        self.freeform_answer(
+            1, mentoring, controls, 'This is a different answer', CORRECT, saved_value='This is the answer'
+        )
         self.single_choice_question(2, mentoring, controls, 'Yes', CORRECT)
         self.rating_question(3, mentoring, controls, "1 - Not good at all", INCORRECT)
 
-        user_selection =  ("Its elegance", "Its beauty", "Its gracefulness")
+        user_selection = ("Its elegance", "Its beauty", "Its gracefulness")
         self.multiple_choice_question(4, mentoring, controls, user_selection, CORRECT, last=True)
 
         expected_results = {
-                "correct": 3, "partial": 0, "incorrect": 1, "percentage": 75,
-                "num_attempts": 2, "max_attempts": 2}
+            "correct": 3, "partial": 0, "incorrect": 1, "percentage": 75,
+            "num_attempts": 2, "max_attempts": 2
+        }
         self.peek_at_review(mentoring, controls, expected_results)
         self.assert_disabled(controls.try_again)
 
@@ -332,7 +329,8 @@ class MentoringAssessmentTest(MentoringBaseTest):
 
         expected_results = {
             "correct": 0, "partial": 0, "incorrect": 1, "percentage": 0,
-            "num_attempts": 1, "max_attempts": 2}
+            "num_attempts": 1, "max_attempts": 2
+        }
 
         self.peek_at_review(mentoring, controls, expected_results)
 

mentoring/tests/unit/test_migration.py

@@ -16,16 +16,16 @@ class TestFieldMigration(unittest.TestCase):
         """
         # Instantiate a mentoring block with the old format
         student_results = [
-            [ u'goal',
-                {   u'completed': True,
-                    u'score': 1,
-                    u'student_input': u'test',
-                    u'weight': 1}],
-            [ u'mcq_1_1',
-                {   u'completed': False,
-                    u'score': 0,
-                    u'submission': u'maybenot',
-                    u'weight': 1}],
+            [u'goal',
+                {u'completed': True,
+                 u'score': 1,
+                 u'student_input': u'test',
+                 u'weight': 1}],
+            [u'mcq_1_1',
+                {u'completed': False,
+                 u'score': 0,
+                 u'submission': u'maybenot',
+                 u'weight': 1}],
         ]
         mentoring = MentoringBlock(MagicMock(), DictFieldData({'student_results': student_results}), Mock())
         self.assertEqual(copy.deepcopy(student_results), mentoring.student_results)

pylintrc

+[REPORTS]
+reports=no
+include-ids=yes
+
+[FORMAT]
+max-line-length=120
+
+[MESSAGES CONTROL]
+disable=invalid-name