From d65d3094cf2f78a397f04ea2e4c2ec4d563451fd Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 4 Dec 2014 17:30:06 -0500 Subject: [PATCH 1/8] Update pep8 to 1.5.7 --- requirements/edx/base.txt | 2 +- setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c4a82a22f5..4ef7496c80 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -127,7 +127,7 @@ mock==1.0.1 nose-exclude nose-ignore-docstring nosexcover==1.0.7 -pep8==1.4.5 +pep8==1.5.7 pylint==0.28 python-subunit==0.0.16 rednose==0.3 diff --git a/setup.cfg b/setup.cfg index f5a7de158e..3ef65d1ece 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,5 +16,5 @@ no-path-adjustment=1 #pdb=1 [pep8] -ignore=E501 +ignore=E501,E265 exclude=migrations From 36e77c746369c3e7df1d70fc82b12721a6fec2f0 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 10 Dec 2014 17:30:03 -0500 Subject: [PATCH 2/8] Fixing pep8 issues --- .../contentstore/features/html-editor.py | 2 +- .../contentstore/tests/test_core_caching.py | 4 +- .../contentstore/views/component.py | 2 +- cms/djangoapps/contentstore/views/course.py | 6 +- cms/envs/test.py | 5 +- .../django_comment_common/models.py | 5 +- common/djangoapps/student/models.py | 40 +- .../student/tests/test_bulk_email_settings.py | 28 +- common/djangoapps/student/views.py | 2 +- common/djangoapps/util/file.py | 2 +- common/lib/capa/capa/correctmap.py | 2 +- common/lib/capa/capa/customrender.py | 2 +- .../lib/capa/capa/tests/test_customrender.py | 1 - .../capa/capa/tests/test_input_templates.py | 38 +- .../lib/capa/capa/tests/test_responsetypes.py | 2 +- common/lib/capa/capa/tests/test_util.py | 4 +- .../sandbox-packages/verifiers/draganddrop.py | 2 +- .../verifiers/tests_draganddrop.py | 46 ++- common/lib/xmodule/xmodule/capa_base.py | 13 +- common/lib/xmodule/xmodule/capa_module.py | 2 +- common/lib/xmodule/xmodule/course_module.py | 72 ++-- common/lib/xmodule/xmodule/html_module.py | 2 +- .../split_mongo/split_mongo_kvs.py | 13 - .../xmodule/modulestore/tests/django_utils.py | 1 - .../test_cross_modulestore_import_export.py | 2 +- .../xmodule/modulestore/tests/test_mongo.py | 1 - .../tests/test_split_modulestore.py | 387 +++++++++--------- .../open_ended_module.py | 6 +- .../xmodule/xmodule/partitions/partitions.py | 2 +- common/lib/xmodule/xmodule/poll_module.py | 2 +- common/lib/xmodule/xmodule/raw_module.py | 11 +- common/lib/xmodule/xmodule/tabs.py | 11 +- .../xmodule/xmodule/tests/test_capa_module.py | 19 +- .../xmodule/tests/test_editing_module.py | 16 +- common/lib/xmodule/xmodule/tests/test_tabs.py | 2 +- .../lib/xmodule/xmodule/word_cloud_module.py | 10 +- .../test/acceptance/pages/studio/container.py | 2 +- .../test/acceptance/pages/studio/overview.py | 2 +- .../tests/studio/test_studio_container.py | 56 ++- .../management/commands/cert_whitelist.py | 10 +- .../courseware/features/problems_setup.py | 2 +- lms/djangoapps/courseware/grades.py | 5 +- lms/djangoapps/courseware/model_data.py | 4 +- .../tests/test_submitting_problems.py | 41 +- .../courseware/tests/test_video_handlers.py | 14 +- .../courseware/tests/test_word_cloud.py | 11 +- lms/djangoapps/dashboard/git_import.py | 2 +- lms/djangoapps/dashboard/sysadmin.py | 4 +- .../django_comment_client/base/views.py | 4 +- .../django_comment_client/forum/views.py | 8 +- .../django_comment_client/permissions.py | 2 +- .../tests/mock_cs_server/mock_cs_server.py | 12 +- lms/djangoapps/instructor/views/api.py | 12 +- .../instructor/views/instructor_dashboard.py | 2 +- lms/djangoapps/instructor/views/legacy.py | 2 +- .../instructor_analytics/distributions.py | 2 +- .../management/commands/create_user.py | 2 +- lms/djangoapps/open_ended_grading/views.py | 4 +- lms/djangoapps/shoppingcart/models.py | 14 +- .../shoppingcart/tests/payment_fake.py | 2 +- lms/envs/edx4edx_aws.py | 4 +- lms/envs/load_test.py | 2 +- lms/envs/test.py | 5 +- 63 files changed, 528 insertions(+), 467 deletions(-) diff --git a/cms/djangoapps/contentstore/features/html-editor.py b/cms/djangoapps/contentstore/features/html-editor.py index 98d57f477d..193af0cf2e 100644 --- a/cms/djangoapps/contentstore/features/html-editor.py +++ b/cms/djangoapps/contentstore/features/html-editor.py @@ -285,7 +285,7 @@ def use_code_editor(action): def perform_action_in_plugin(action): - # Wait for the plugin window to open. + # Wait for the plugin window to open. world.wait_for_visible('.mce-window') # Trigger the action diff --git a/cms/djangoapps/contentstore/tests/test_core_caching.py b/cms/djangoapps/contentstore/tests/test_core_caching.py index 16b15d127f..7470ac1d9d 100644 --- a/cms/djangoapps/contentstore/tests/test_core_caching.py +++ b/cms/djangoapps/contentstore/tests/test_core_caching.py @@ -13,7 +13,9 @@ class Content: class CachingTestCase(TestCase): -# Tests for https://edx.lighthouseapp.com/projects/102637/tickets/112-updating-asset-does-not-refresh-the-cached-copy + """ + Tests for https://edx.lighthouseapp.com/projects/102637/tickets/112-updating-asset-does-not-refresh-the-cached-copy + """ unicodeLocation = Location(u'c4x', u'mitX', u'800', u'run', u'thumbnail', u'monsters.jpg') # Note that some of the parts are strings instead of unicode strings nonUnicodeLocation = Location('c4x', u'mitX', u'800', u'run', 'thumbnail', 'monsters.jpg') diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 04e513b1cd..7904746009 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -303,7 +303,7 @@ def get_component_templates(course): # Set component types according to course policy file if isinstance(course_advanced_keys, list): for category in course_advanced_keys: - if category in advanced_component_types and not category in categories: + if category in advanced_component_types and category not in categories: # boilerplates not supported for advanced components try: component_display_name = xblock_type_display_name(category, default_display_name=category) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ba14898e69..6548607eb1 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1028,7 +1028,7 @@ def textbooks_list_handler(request, course_key_string): with store.bulk_operations(course_key): course = _get_course_module(course_key, request.user) - if not "application/json" in request.META.get('HTTP_ACCEPT', 'text/html'): + if "application/json" not in request.META.get('HTTP_ACCEPT', 'text/html'): # return HTML page upload_asset_url = reverse_course_url('assets_handler', course_key) textbook_url = reverse_course_url('textbooks_list_handler', course_key) @@ -1050,7 +1050,7 @@ def textbooks_list_handler(request, course_key_string): tids = set(t["id"] for t in textbooks if "id" in t) for textbook in textbooks: - if not "id" in textbook: + if "id" not in textbook: tid = assign_textbook_id(textbook, tids) textbook["id"] = tid tids.add(tid) @@ -1347,7 +1347,7 @@ def group_configurations_list_handler(request, course_key_string): }) elif "application/json" in request.META.get('HTTP_ACCEPT'): if request.method == 'POST': - # create a new group configuration for the course + # create a new group configuration for the course try: new_configuration = GroupConfiguration(request.body, course).get_user_partition() except GroupConfigurationsValidationError as err: diff --git a/cms/envs/test.py b/cms/envs/test.py index f301d35b79..9e3d760dff 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -158,8 +158,9 @@ filterwarnings('ignore', message='No request passed to the backend, unable to ra # Ignore deprecation warnings (so we don't clutter Jenkins builds/production) # https://docs.python.org/2/library/warnings.html#the-warnings-filter -simplefilter('ignore') # Change to "default" to see the first instance of each hit - # or "error" to convert all into errors +simplefilter('ignore') +# Change to "default" to see the first instance of each hit +# or "error" to convert all into errors ################################# CELERY ###################################### diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index 43d80eb98f..feba43a0eb 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -64,8 +64,9 @@ class Role(models.Model): # pylint: disable=no-member return self.name + " for " + (self.course_id.to_deprecated_string() if self.course_id else "all courses") - def inherit_permissions(self, role): # TODO the name of this method is a little bit confusing, - # since it's one-off and doesn't handle inheritance later + # TODO the name of this method is a little bit confusing, + # since it's one-off and doesn't handle inheritance later + def inherit_permissions(self, role): if role.course_id and role.course_id != self.course_id: logging.warning( "%s cannot inherit permissions from %s due to course_id inconsistency", diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 407701cace..a85caeda08 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -452,50 +452,50 @@ class PasswordHistory(models.Model): """ Returns whether the configuration which limits password reuse has been turned on """ - return settings.FEATURES['ADVANCED_SECURITY'] and \ - settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE', 0 - ) > 0 + min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( + 'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE', 0 + ) + return settings.FEATURES['ADVANCED_SECURITY'] and min_diff_pw > 0 @classmethod def is_staff_password_reuse_restricted(cls): """ Returns whether the configuration which limits password reuse has been turned on """ - return settings.FEATURES['ADVANCED_SECURITY'] and \ - settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE', 0 - ) > 0 + min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( + 'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE', 0 + ) + return settings.FEATURES['ADVANCED_SECURITY'] and min_diff_pw > 0 @classmethod def is_password_reset_frequency_restricted(cls): """ Returns whether the configuration which limits the password reset frequency has been turned on """ - return settings.FEATURES['ADVANCED_SECURITY'] and \ - settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS', None - ) + min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( + 'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS' + ) + return settings.FEATURES['ADVANCED_SECURITY'] and min_days_between_reset @classmethod def is_staff_forced_password_reset_enabled(cls): """ Returns whether the configuration which forces password resets to occur has been turned on """ - return settings.FEATURES['ADVANCED_SECURITY'] and \ - settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS', None - ) + min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( + 'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS' + ) + return settings.FEATURES['ADVANCED_SECURITY'] and min_days_between_reset @classmethod def is_student_forced_password_reset_enabled(cls): """ Returns whether the configuration which forces password resets to occur has been turned on """ - return settings.FEATURES['ADVANCED_SECURITY'] and \ - settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS', None - ) + min_days_pw_reset = settings.ADVANCED_SECURITY_CONFIG.get( + 'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS' + ) + return settings.FEATURES['ADVANCED_SECURITY'] and min_days_pw_reset @classmethod def should_user_reset_password_now(cls, user): diff --git a/common/djangoapps/student/tests/test_bulk_email_settings.py b/common/djangoapps/student/tests/test_bulk_email_settings.py index 4410320aae..b3390a5790 100644 --- a/common/djangoapps/student/tests/test_bulk_email_settings.py +++ b/common/djangoapps/student/tests/test_bulk_email_settings.py @@ -41,13 +41,13 @@ class TestStudentDashboardEmailView(ModuleStoreTestCase): self.url = reverse('dashboard') # URL for email settings modal self.email_modal_link = ( - ('').format( - self.course.org, - self.course.number, - self.course.display_name.replace(' ', '_') - ) + '' + ).format( + org=self.course.org, + num=self.course.number, + name=self.course.display_name.replace(' ', '_'), ) def tearDown(self): @@ -111,13 +111,13 @@ class TestStudentDashboardEmailViewXMLBacked(ModuleStoreTestCase): # URL for email settings modal self.email_modal_link = ( - ('').format( - 'edX', - 'toy', - '2012_Fall' - ) + '' + ).format( + org='edX', + num='toy', + name='2012_Fall', ) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 8b6292eecf..b25678e3ec 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1002,7 +1002,7 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un _("Use your {platform_name} username and password to log into {platform_name} below, " "and then link your {platform_name} account with {provider_name} from your dashboard.").format( platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME - ) + ) + "

" + _("If you don't have an {platform_name} account yet, click Register Now at the top of the page.").format( platform_name=settings.PLATFORM_NAME diff --git a/common/djangoapps/util/file.py b/common/djangoapps/util/file.py index e4e2efe50d..668ed59fcf 100644 --- a/common/djangoapps/util/file.py +++ b/common/djangoapps/util/file.py @@ -55,7 +55,7 @@ def store_uploaded_file( uploaded_file = request.FILES[file_key] try: file_extension = os.path.splitext(uploaded_file.name)[1].lower() - if not file_extension in allowed_file_types: + if file_extension not in allowed_file_types: file_types = "', '".join(allowed_file_types) msg = ungettext( "The file must end with the extension '{file_types}'.", diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index ff6e9ed142..e470ac294e 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -122,7 +122,7 @@ class CorrectMap(object): return npoints elif self.is_correct(answer_id): return 1 - # if not correct and no points have been assigned, return 0 + # if not correct and no points have been assigned, return 0 return 0 def set_property(self, answer_id, property, value): diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py index f56f963577..bc80d270f4 100644 --- a/common/lib/capa/capa/customrender.py +++ b/common/lib/capa/capa/customrender.py @@ -44,7 +44,7 @@ class MathRenderer(object): mathstr = re.sub(r'\$(.*)\$', r'[mathjaxinline]\1[/mathjaxinline]', xml.text) mtag = 'mathjax' - if not r'\displaystyle' in mathstr: + if r'\displaystyle' not in mathstr: mtag += 'inline' else: mathstr = mathstr.replace(r'\displaystyle', '') diff --git a/common/lib/capa/capa/tests/test_customrender.py b/common/lib/capa/capa/tests/test_customrender.py index 8eeda47333..9cb1de7d9b 100644 --- a/common/lib/capa/capa/tests/test_customrender.py +++ b/common/lib/capa/capa/tests/test_customrender.py @@ -74,5 +74,4 @@ class MathRenderTest(unittest.TestCase): self.check_parse('$abc', '$abc') self.check_parse(r'$\displaystyle 2+2$', '[mathjax] 2+2[/mathjax]') - # NOTE: not testing get_html yet because I don't understand why it's doing what it's doing. diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 2dc90237c6..2fcc29e301 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -795,17 +795,30 @@ class ChoiceTextGroupTemplateTest(TemplateTestCase): '1_choiceinput_1_textinput_0': '0'} def setUp(self): - choices = [('1_choiceinput_0bc', - [{'tail_text': '', 'type': 'text', 'value': '', 'contents': ''}, - {'tail_text': '', 'type': 'textinput', 'value': '', 'contents': 'choiceinput_0_textinput_0'}]), - ('1_choiceinput_1bc', [{'tail_text': '', 'type': 'text', 'value': '', 'contents': ''}, - {'tail_text': '', 'type': 'textinput', 'value': '', 'contents': 'choiceinput_1_textinput_0'}])] - self.context = {'id': '1', - 'choices': choices, - 'status': Status('correct'), - 'input_type': 'radio', - 'label': 'choicetext label', - 'value': self.VALUE_DICT} + choices = [ + ( + '1_choiceinput_0bc', + [ + {'tail_text': '', 'type': 'text', 'value': '', 'contents': ''}, + {'tail_text': '', 'type': 'textinput', 'value': '', 'contents': 'choiceinput_0_textinput_0'}, + ] + ), + ( + '1_choiceinput_1bc', + [ + {'tail_text': '', 'type': 'text', 'value': '', 'contents': ''}, + {'tail_text': '', 'type': 'textinput', 'value': '', 'contents': 'choiceinput_1_textinput_0'}, + ] + ) + ] + self.context = { + 'id': '1', + 'choices': choices, + 'status': Status('correct'), + 'input_type': 'radio', + 'label': 'choicetext label', + 'value': self.VALUE_DICT, + } super(ChoiceTextGroupTemplateTest, self).setUp() @@ -885,7 +898,8 @@ class ChoiceTextGroupTemplateTest(TemplateTestCase): {'status': Status('unsubmitted'), 'input_type': 'checkbox', 'value': {}}, {'status': Status('unsubmitted'), 'input_type': 'checkbox', 'value': self.EMPTY_DICT}, {'status': Status('unsubmitted'), 'input_type': 'checkbox', 'value': self.VALUE_DICT}, - {'status': Status('unsubmitted'), 'input_type': 'checkbox', 'value': self.BOTH_CHOICE_CHECKBOX}] + {'status': Status('unsubmitted'), 'input_type': 'checkbox', 'value': self.BOTH_CHOICE_CHECKBOX}, + ] self.context['status'] = Status('unanswered') diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 970e3ea58e..4068b8eed6 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -731,7 +731,7 @@ class StringResponseTest(ResponseTest): case_sensitive=False, hints=hints, ) - # We should get a hint for Wisconsin + # We should get a hint for Wisconsin input_dict = {'1_2_1': 'Wisconsin'} correct_map = problem.grade_answers(input_dict) self.assertEquals(correct_map.get_hint('1_2_1'), diff --git a/common/lib/capa/capa/tests/test_util.py b/common/lib/capa/capa/tests/test_util.py index 87ef04eaaa..f6b76e1c9e 100644 --- a/common/lib/capa/capa/tests/test_util.py +++ b/common/lib/capa/capa/tests/test_util.py @@ -36,7 +36,7 @@ class UtilTest(unittest.TestCase): self.assertTrue(result) result = compare_with_tolerance(110.1, 100.0, '10.0', False) self.assertFalse(result) - # Test relative tolerance (string) + # Test relative tolerance (string) result = compare_with_tolerance(111.0, 100.0, '0.1', True) self.assertTrue(result) result = compare_with_tolerance(112.0, 100.0, '0.1', True) @@ -46,7 +46,7 @@ class UtilTest(unittest.TestCase): self.assertTrue(result) result = compare_with_tolerance(110.1, 100.0, 10.0, False) self.assertFalse(result) - # Test relative tolerance (float) + # Test relative tolerance (float) result = compare_with_tolerance(111.0, 100.0, 0.1, True) self.assertTrue(result) result = compare_with_tolerance(112.0, 100.0, 0.1, True) diff --git a/common/lib/sandbox-packages/verifiers/draganddrop.py b/common/lib/sandbox-packages/verifiers/draganddrop.py index 3a00478a7d..bce588ac42 100644 --- a/common/lib/sandbox-packages/verifiers/draganddrop.py +++ b/common/lib/sandbox-packages/verifiers/draganddrop.py @@ -191,7 +191,7 @@ class DragAndDrop(object): self.user_positions[index]['user'], flag=rule): return False if not rules_executed: # no correct rules for current group - # probably xml content mistake - wrong rules names + # probably xml content mistake - wrong rules names return False return True diff --git a/common/lib/sandbox-packages/verifiers/tests_draganddrop.py b/common/lib/sandbox-packages/verifiers/tests_draganddrop.py index 9ab6115037..8a27dd9287 100644 --- a/common/lib/sandbox-packages/verifiers/tests_draganddrop.py +++ b/common/lib/sandbox-packages/verifiers/tests_draganddrop.py @@ -90,15 +90,16 @@ class Test_DragAndDrop_Grade(unittest.TestCase): }, { 'draggables': ['up_and_down'], - 'targets': [ - 's_l[s][1]', 's_r[s][1]' - ], + 'targets': ['s_l[s][1]', 's_r[s][1]'], 'rule': 'unordered_equal' }, { 'draggables': ['up'], 'targets': [ - 'p_l[p][1]', 'p_l[p][3]', 'p_r[p][1]', 'p_r[p][3]' + 'p_l[p][1]', + 'p_l[p][3]', + 'p_r[p][1]', + 'p_r[p][3]', ], 'rule': 'unordered_equal' } @@ -132,15 +133,16 @@ class Test_DragAndDrop_Grade(unittest.TestCase): }, { 'draggables': ['up_and_down'], - 'targets': [ - 's_l[s][1]', 's_r[s][1]' - ], + 'targets': ['s_l[s][1]', 's_r[s][1]'], 'rule': 'unordered_equal' }, { 'draggables': ['up'], 'targets': [ - 'p_l[p][1]', 'p_l[p][3]', 'p_r[p][1]', 'p_r[p][3]' + 'p_l[p][1]', + 'p_l[p][3]', + 'p_r[p][1]', + 'p_r[p][3]', ], 'rule': 'unordered_equal' } @@ -173,7 +175,7 @@ class Test_DragAndDrop_Grade(unittest.TestCase): 'draggables': ['p'], 'targets': [ 'left_side_tagret[molecule][p_target]', - 'right_side_tagret[molecule][p_target]' + 'right_side_tagret[molecule][p_target]', ], 'rule': 'unordered_equal' }, @@ -181,7 +183,7 @@ class Test_DragAndDrop_Grade(unittest.TestCase): 'draggables': ['s'], 'targets': [ 'left_side_tagret[molecule][s_target]', - 'right_side_tagret[molecule][s_target]' + 'right_side_tagret[molecule][s_target]', ], 'rule': 'unordered_equal' }, @@ -189,7 +191,7 @@ class Test_DragAndDrop_Grade(unittest.TestCase): 'draggables': ['up_and_down'], 'targets': [ 'left_side_tagret[molecule][s_target][s][1]', - 'right_side_tagret[molecule][s_target][s][1]' + 'right_side_tagret[molecule][s_target][s][1]', ], 'rule': 'unordered_equal' }, @@ -199,7 +201,8 @@ class Test_DragAndDrop_Grade(unittest.TestCase): 'left_side_tagret[molecule][p_target][p][1]', 'left_side_tagret[molecule][p_target][p][3]', 'right_side_tagret[molecule][p_target][p][1]', - 'right_side_tagret[molecule][p_target][p][3]'], + 'right_side_tagret[molecule][p_target][p][3]', + ], 'rule': 'unordered_equal' } ] @@ -255,15 +258,24 @@ class Test_DragAndDrop_Grade(unittest.TestCase): }, { 'draggables': ['up'], - 'targets': ['p_l[triple_draggable][1]', 'p_l[triple_draggable][2]', - 'p_r[triple_draggable][2]', 'p_r[triple_draggable][3]'], + 'targets': [ + 'p_l[triple_draggable][1]', + 'p_l[triple_draggable][2]', + 'p_r[triple_draggable][2]', + 'p_r[triple_draggable][3]', + ], 'rule': 'unordered_equal' }, { 'draggables': ['up_and_down'], - 'targets': ['s_l[single_draggable][1]', 's_r[single_draggable][1]', - 's_sigma[single_draggable][1]', 's_sigma*[single_draggable][1]', - 'p_pi[double_draggable][1]', 'p_pi[double_draggable][2]'], + 'targets': [ + 's_l[single_draggable][1]', + 's_r[single_draggable][1]', + 's_sigma[single_draggable][1]', + 's_sigma*[single_draggable][1]', + 'p_pi[double_draggable][1]', + 'p_pi[double_draggable][2]', + ], 'rule': 'unordered_equal' }, diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 72fd88972b..a3fca52714 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -263,8 +263,11 @@ class CapaMixin(CapaFields): msg += u'

{tb}

'.format( # just the traceback, no message - it is already present above tb=cgi.escape( - u''.join(['Traceback (most recent call last):\n'] + - traceback.format_tb(sys.exc_info()[2]))) + u''.join( + ['Traceback (most recent call last):\n'] + + traceback.format_tb(sys.exc_info()[2]) + ) + ) ) # create a dummy problem with error message instead of failing problem_text = (u'' @@ -987,8 +990,8 @@ class CapaMixin(CapaFields): # Wait time between resets: check if is too soon for submission. if self.last_submission_time is not None and self.submission_wait_seconds != 0: - # pylint: disable=maybe-no-member - # pylint is unable to verify that .total_seconds() exists + # pylint: disable=maybe-no-member + # pylint is unable to verify that .total_seconds() exists if (current_time - self.last_submission_time).total_seconds() < self.submission_wait_seconds: remaining_secs = int(self.submission_wait_seconds - (current_time - self.last_submission_time).total_seconds()) msg = _(u'You must wait at least {wait_secs} between submissions. {remaining_secs} remaining.').format( @@ -1126,7 +1129,7 @@ class CapaMixin(CapaFields): if permutation_option is not None: # Add permutation record tuple: (one of:'shuffle'/'answerpool', [as-displayed list]) - if not 'permutation' in event_info: + if 'permutation' not in event_info: event_info['permutation'] = {} event_info['permutation'][response.answer_id] = (permutation_option, response.unmask_order()) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index dc46017a27..776dc36e3f 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -138,7 +138,7 @@ class CapaDescriptor(CapaFields, RawDescriptor): Show them only if use_latex_compiler is set to True in course settings. """ - return (not 'latex' in template['template_id'] or course.use_latex_compiler) + return ('latex' not in template['template_id'] or course.use_latex_compiler) def get_context(self): _context = RawDescriptor.get_context(self) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 48b3b89227..c58535c0e4 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -184,40 +184,44 @@ class CourseFields(object): help=_("Enter the date you want to advertise as the course start date, if this date is different from the set start date. To advertise the set start date, enter null."), scope=Scope.settings ) - grading_policy = Dict(help="Grading policy definition for this class", - default={"GRADER": [ - { - "type": "Homework", - "min_count": 12, - "drop_count": 2, - "short_label": "HW", - "weight": 0.15 - }, - { - "type": "Lab", - "min_count": 12, - "drop_count": 2, - "weight": 0.15 - }, - { - "type": "Midterm Exam", - "short_label": "Midterm", - "min_count": 1, - "drop_count": 0, - "weight": 0.3 - }, - { - "type": "Final Exam", - "short_label": "Final", - "min_count": 1, - "drop_count": 0, - "weight": 0.4 - } - ], - "GRADE_CUTOFFS": { - "Pass": 0.5 - }}, - scope=Scope.content) + grading_policy = Dict( + help="Grading policy definition for this class", + default={ + "GRADER": [ + { + "type": "Homework", + "min_count": 12, + "drop_count": 2, + "short_label": "HW", + "weight": 0.15, + }, + { + "type": "Lab", + "min_count": 12, + "drop_count": 2, + "weight": 0.15, + }, + { + "type": "Midterm Exam", + "short_label": "Midterm", + "min_count": 1, + "drop_count": 0, + "weight": 0.3, + }, + { + "type": "Final Exam", + "short_label": "Final", + "min_count": 1, + "drop_count": 0, + "weight": 0.4, + } + ], + "GRADE_CUTOFFS": { + "Pass": 0.5, + }, + }, + scope=Scope.content + ) show_calculator = Boolean( display_name=_("Show Calculator"), help=_("Enter true or false. When true, students can see the calculator in the course."), diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 36167b971f..ef14ddfe28 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -113,7 +113,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): Show them only if use_latex_compiler is set to True in course settings. """ - return (not 'latex' in template['template_id'] or course.use_latex_compiler) + return ('latex' not in template['template_id'] or course.use_latex_compiler) def get_context(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 3a7766881e..8b90ee47f9 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -76,13 +76,6 @@ class SplitMongoKVS(InheritanceKeyValueStore): # set the field self._fields[key.field_name] = value - # handle any side effects -- story STUD-624 - # if key.scope == Scope.children: - # STUD-624 remove inheritance from any exchildren - # STUD-624 add inheritance to any new children - # if key.scope == Scope.settings: - # STUD-624 if inheritable, push down to children - def delete(self, key): # handle any special cases if key.scope not in [Scope.children, Scope.settings, Scope.content]: @@ -94,12 +87,6 @@ class SplitMongoKVS(InheritanceKeyValueStore): if key.field_name in self._fields: del self._fields[key.field_name] - # handle any side effects - # if key.scope == Scope.children: - # STUD-624 remove inheritance from any exchildren - # if key.scope == Scope.settings: - # STUD-624 if inheritable, push down _inherited_settings value to children - def has(self, key): """ Is the given field explicitly set in this kvs (not inherited nor default) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 5c0eafc319..9bc9c40647 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -336,7 +336,6 @@ class ModuleStoreTestCase(TestCase): 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 # pylint: disable=attribute-defined-outside-init diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index ca3a10a3b1..82d469612d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -277,7 +277,7 @@ MIXED_MODULESTORE_SETUPS = ( ) DIRECT_MODULESTORE_SETUPS = ( MongoModulestoreBuilder(), -# VersioningModulestoreBuilder(), # FUTUREDO: LMS-11227 + # VersioningModulestoreBuilder(), # FUTUREDO: LMS-11227 ) MODULESTORE_SETUPS = DIRECT_MODULESTORE_SETUPS + MIXED_MODULESTORE_SETUPS diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 270114a25f..a6f1dd5aa7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -89,7 +89,6 @@ class TestMongoModuleStoreBase(unittest.TestCase): @classmethod def teardownClass(cls): -# cls.patcher.stop() if cls.connection: cls.connection.drop_database(DB) cls.connection.close() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 4b399de147..14e80b965a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -132,168 +132,172 @@ class SplitModuleTest(unittest.TestCase): }, }, }, - "revisions": [{ - "user_id": "testassist@edx.org", - "update": { - ("course", "head12345"): { - "end": _date_field.from_json("2013-04-13T04:30"), - "tabs": [ - { - "type": "courseware" - }, - { - "type": "course_info", - "name": "Course Info" - }, - { - "type": "discussion", - "name": "Discussion" - }, - { - "type": "wiki", - "name": "Wiki" - }, - { - "type": "static_tab", - "name": "Syllabus", - "url_slug": "01356a17b5924b17a04b7fc2426a3798" - }, - { - "type": "static_tab", - "name": "Advice for Students", - "url_slug": "57e9991c0d794ff58f7defae3e042e39" - } - ], - "graceperiod": _time_delta_field.from_json("2 hours 0 minutes 0 seconds"), - "grading_policy": { - "GRADER": [ + "revisions": [ + { + "user_id": "testassist@edx.org", + "update": { + ("course", "head12345"): { + "end": _date_field.from_json("2013-04-13T04:30"), + "tabs": [ { - "min_count": 5, - "weight": 0.15, - "type": "Homework", - "drop_count": 1, - "short_label": "HWa" + "type": "courseware" }, { - "short_label": "", - "min_count": 12, - "type": "Lab", - "drop_count": 2, - "weight": 0.15 + "type": "course_info", + "name": "Course Info" }, { - "short_label": "Midterm", - "min_count": 1, - "type": "Midterm Exam", - "drop_count": 0, - "weight": 0.3 + "type": "discussion", + "name": "Discussion" }, { - "short_label": "Final", - "min_count": 1, - "type": "Final Exam", - "drop_count": 0, - "weight": 0.4 + "type": "wiki", + "name": "Wiki" + }, + { + "type": "static_tab", + "name": "Syllabus", + "url_slug": "01356a17b5924b17a04b7fc2426a3798" + }, + { + "type": "static_tab", + "name": "Advice for Students", + "url_slug": "57e9991c0d794ff58f7defae3e042e39" } ], - "GRADE_CUTOFFS": { - "Pass": 0.55 - } - }, - }} - }, - {"user_id": "testassist@edx.org", - "update": - {("course", "head12345"): { - "end": _date_field.from_json("2013-06-13T04:30"), - "grading_policy": { - "GRADER": [ - { - "min_count": 4, - "weight": 0.15, - "type": "Homework", - "drop_count": 2, - "short_label": "HWa" - }, - { - "short_label": "", - "min_count": 12, - "type": "Lab", - "drop_count": 2, - "weight": 0.15 - }, - { - "short_label": "Midterm", - "min_count": 1, - "type": "Midterm Exam", - "drop_count": 0, - "weight": 0.3 - }, - { - "short_label": "Final", - "min_count": 1, - "type": "Final Exam", - "drop_count": 0, - "weight": 0.4 + "graceperiod": _time_delta_field.from_json("2 hours 0 minutes 0 seconds"), + "grading_policy": { + "GRADER": [ + { + "min_count": 5, + "weight": 0.15, + "type": "Homework", + "drop_count": 1, + "short_label": "HWa" + }, + { + "short_label": "", + "min_count": 12, + "type": "Lab", + "drop_count": 2, + "weight": 0.15 + }, + { + "short_label": "Midterm", + "min_count": 1, + "type": "Midterm Exam", + "drop_count": 0, + "weight": 0.3 + }, + { + "short_label": "Final", + "min_count": 1, + "type": "Final Exam", + "drop_count": 0, + "weight": 0.4 + } + ], + "GRADE_CUTOFFS": { + "Pass": 0.55 } - ], - "GRADE_CUTOFFS": { - "Pass": 0.45 - } + }, + } + } + }, + { + "user_id": "testassist@edx.org", + "update": { + ("course", "head12345"): { + "end": _date_field.from_json("2013-06-13T04:30"), + "grading_policy": { + "GRADER": [ + { + "min_count": 4, + "weight": 0.15, + "type": "Homework", + "drop_count": 2, + "short_label": "HWa" + }, + { + "short_label": "", + "min_count": 12, + "type": "Lab", + "drop_count": 2, + "weight": 0.15 + }, + { + "short_label": "Midterm", + "min_count": 1, + "type": "Midterm Exam", + "drop_count": 0, + "weight": 0.3 + }, + { + "short_label": "Final", + "min_count": 1, + "type": "Final Exam", + "drop_count": 0, + "weight": 0.4 + } + ], + "GRADE_CUTOFFS": { + "Pass": 0.45 + } + }, + "enrollment_start": _date_field.from_json("2013-01-01T05:00"), + "enrollment_end": _date_field.from_json("2013-03-02T05:00"), + "advertised_start": "Fall 2013", + } + }, + "create": [ + { + "id": "chapter1", + "parent": "head12345", + "parent_type": "course", + "category": "chapter", + "fields": { + "display_name": "Hercules" + }, }, - "enrollment_start": _date_field.from_json("2013-01-01T05:00"), - "enrollment_end": _date_field.from_json("2013-03-02T05:00"), - "advertised_start": "Fall 2013", - }}, - "create": [ - { - "id": "chapter1", - "parent": "head12345", - "parent_type": "course", - "category": "chapter", - "fields": { - "display_name": "Hercules" - }, - }, - { - "id": "chapter2", - "parent": "head12345", - "parent_type": "course", - "category": "chapter", - "fields": { - "display_name": "Hera heckles Hercules" - }, - }, - { - "id": "chapter3", - "parent": "head12345", - "parent_type": "course", - "category": "chapter", - "fields": { - "display_name": "Hera cuckolds Zeus" - }, - }, - { - "id": "problem1", - "parent": "chapter3", - "parent_type": "chapter", - "category": "problem", - "fields": { - "display_name": "Problem 3.1", - "graceperiod": _time_delta_field.from_json("4 hours 0 minutes 0 seconds"), - }, - }, - { - "id": "problem3_2", - "parent": "chapter3", - "parent_type": "chapter", - "category": "problem", - "fields": { - "display_name": "Problem 3.2" - }, - } - ] - }, + { + "id": "chapter2", + "parent": "head12345", + "parent_type": "course", + "category": "chapter", + "fields": { + "display_name": "Hera heckles Hercules" + }, + }, + { + "id": "chapter3", + "parent": "head12345", + "parent_type": "course", + "category": "chapter", + "fields": { + "display_name": "Hera cuckolds Zeus" + }, + }, + { + "id": "problem1", + "parent": "chapter3", + "parent_type": "chapter", + "category": "problem", + "fields": { + "display_name": "Problem 3.1", + "graceperiod": _time_delta_field.from_json("4 hours 0 minutes 0 seconds"), + }, + }, + { + "id": "problem3_2", + "parent": "chapter3", + "parent_type": "chapter", + "category": "problem", + "fields": { + "display_name": "Problem 3.2" + }, + } + ] + }, ] }, "testx.wonderful": { @@ -358,49 +362,50 @@ class SplitModuleTest(unittest.TestCase): } }, }, - "revisions": [{ - "user_id": "test@edx.org", - "update": { - ("course", "head23456"): { - "display_name": "The most wonderful course", - "grading_policy": { - "GRADER": [ - { - "min_count": 14, - "weight": 0.25, - "type": "Homework", - "drop_count": 1, - "short_label": "HWa" - }, - { - "short_label": "", - "min_count": 12, - "type": "Lab", - "drop_count": 2, - "weight": 0.25 - }, - { - "short_label": "Midterm", - "min_count": 1, - "type": "Midterm Exam", - "drop_count": 0, - "weight": 0.2 - }, - { - "short_label": "Final", - "min_count": 1, - "type": "Final Exam", - "drop_count": 0, - "weight": 0.3 + "revisions": [ + { + "user_id": "test@edx.org", + "update": { + ("course", "head23456"): { + "display_name": "The most wonderful course", + "grading_policy": { + "GRADER": [ + { + "min_count": 14, + "weight": 0.25, + "type": "Homework", + "drop_count": 1, + "short_label": "HWa" + }, + { + "short_label": "", + "min_count": 12, + "type": "Lab", + "drop_count": 2, + "weight": 0.25 + }, + { + "short_label": "Midterm", + "min_count": 1, + "type": "Midterm Exam", + "drop_count": 0, + "weight": 0.2 + }, + { + "short_label": "Final", + "min_count": 1, + "type": "Final Exam", + "drop_count": 0, + "weight": 0.3 + } + ], + "GRADE_CUTOFFS": { + "Pass": 0.45 } - ], - "GRADE_CUTOFFS": { - "Pass": 0.45 - } - }, + }, + } } } - } ] }, "guestx.contender": { diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 6ea21c1fef..78645816aa 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -329,16 +329,16 @@ class OpenEndedModule(openendedchild.OpenEndedChild): self.record_latest_post_assessment(score_msg) self.child_state = self.POST_ASSESSMENT else: - log.error(( + log.error( "Trying to update score without existing studentmodule child_history:\n" " location: {location}\n" " score: {score}\n" " grader_ids: {grader_ids}\n" - " submission_ids: {submission_ids}").format( + " submission_ids: {submission_ids}".format( location=self.location_string, score=new_score_msg['score'], grader_ids=new_score_msg['grader_ids'], - submission_ids=new_score_msg['submission_ids'] + submission_ids=new_score_msg['submission_ids'], ) ) diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py index 27b6f7a75f..715bbb0338 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions.py +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -150,7 +150,7 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche # If no scheme was provided, set it to the default ('random') scheme_id = UserPartition.VERSION_1_SCHEME elif value["version"] == UserPartition.VERSION: - if not "scheme" in value: + if "scheme" not in value: raise TypeError("UserPartition dict {0} missing value key 'scheme'".format(value)) scheme_id = value["scheme"] else: diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index f17af2e71c..0f6235dc1e 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -121,7 +121,7 @@ class PollModule(PollFields, XModule): # Now we use this hack. temp_poll_answers = self.poll_answers - # Fill self.poll_answers, prepare data for template context. + # Fill self.poll_answers, prepare data for template context. for answer in self.answers: # Set default count for answer = 0. if answer['id'] not in temp_poll_answers: diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index c0cb258254..0e9087c540 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -27,10 +27,13 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor): # re-raise lines = self.data.split('\n') line, offset = err.position - msg = (u"Unable to create xml for module {loc}. " - "Context: '{context}'".format( - context=lines[line - 1][offset - 40:offset + 40], - loc=self.location)) + msg = ( + u"Unable to create xml for module {loc}. " + u"Context: '{context}'" + ).format( + context=lines[line - 1][offset - 40:offset + 40], + loc=self.location, + ) raise SerializationError(self.location, msg) diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 5ad8f58c48..b5d64a6a6e 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -870,10 +870,13 @@ class CourseTabList(List): """ count = sum(1 for tab in tabs if tab.get('type') == tab_type) if count > max_num: - raise InvalidTabsException( - "Tab of type '{0}' appears {1} time(s). Expected maximum of {2} time(s).".format( - tab_type, count, max_num - )) + msg = ( + "Tab of type '{type}' appears {count} time(s). " + "Expected maximum of {max} time(s)." + ).format( + type=tab_type, count=count, max=max_num, + ) + raise InvalidTabsException(msg) def to_json(self, values): """ diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 65213e58f0..c829e29654 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -13,7 +13,7 @@ import textwrap import unittest import ddt -from mock import Mock, patch +from mock import Mock, patch, DEFAULT import webob from webob.multidict import MultiDict @@ -582,19 +582,20 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(attempts=1) # Simulate that the problem is queued - with patch('capa.capa_problem.LoncapaProblem.is_queued') \ - as mock_is_queued, \ - patch('capa.capa_problem.LoncapaProblem.get_recentmost_queuetime') \ - as mock_get_queuetime: - - mock_is_queued.return_value = True - mock_get_queuetime.return_value = datetime.datetime.now(UTC) + multipatch = patch.multiple( + 'capa.capa_problem.LoncapaProblem', + is_queued=DEFAULT, + get_recentmost_queuetime=DEFAULT + ) + with multipatch as values: + values['is_queued'].return_value = True + values['get_recentmost_queuetime'].return_value = datetime.datetime.now(UTC) get_request_dict = {CapaFactory.input_key(): '3.14'} result = module.check_problem(get_request_dict) # Expect an AJAX alert message in 'success' - self.assertTrue('You must wait' in result['success']) + self.assertIn('You must wait', result['success']) # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py index b492cf63b1..14578ca197 100644 --- a/common/lib/xmodule/xmodule/tests/test_editing_module.py +++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py @@ -28,10 +28,18 @@ class TabsEditingDescriptorTestCase(unittest.TestCase): 'template': "tabs/codemirror-edit.html", 'current': True, 'css': { - 'scss': [resource_string(__name__, - '../../test_files/test_tabseditingdescriptor.scss')], - 'css': [resource_string(__name__, - '../../test_files/test_tabseditingdescriptor.css')] + 'scss': [ + resource_string( + __name__, + '../../test_files/test_tabseditingdescriptor.scss' + ) + ], + 'css': [ + resource_string( + __name__, + '../../test_files/test_tabseditingdescriptor.css' + ) + ] } }, { diff --git a/common/lib/xmodule/xmodule/tests/test_tabs.py b/common/lib/xmodule/xmodule/tests/test_tabs.py index 537f1fb2e0..7e1ed5964b 100644 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ b/common/lib/xmodule/xmodule/tests/test_tabs.py @@ -580,7 +580,7 @@ class CourseTabListTestCase(TabListTestCase): )): self.assertEquals(tab.type, self.course.tabs[i].type) - # enumerate the tabs and verify textbooks and the instructor tab + # enumerate the tabs and verify textbooks and the instructor tab for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( self.course, self.settings, diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index e4e615c0f2..ab00abea0e 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -86,10 +86,12 @@ class WordCloudModule(WordCloudFields, XModule): """WordCloud Xmodule""" js = { 'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee')], - 'js': [resource_string(__name__, 'js/src/word_cloud/d3.min.js'), - resource_string(__name__, 'js/src/word_cloud/d3.layout.cloud.js'), - resource_string(__name__, 'js/src/word_cloud/word_cloud.js'), - resource_string(__name__, 'js/src/word_cloud/word_cloud_main.js')] + 'js': [ + resource_string(__name__, 'js/src/word_cloud/d3.min.js'), + resource_string(__name__, 'js/src/word_cloud/d3.layout.cloud.js'), + resource_string(__name__, 'js/src/word_cloud/word_cloud.js'), + resource_string(__name__, 'js/src/word_cloud/word_cloud_main.js'), + ], } css = {'scss': [resource_string(__name__, 'css/word_cloud/display.scss')]} js_module_name = "WordCloud" diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 878a0e0bbf..81f7c7dab7 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -331,7 +331,7 @@ class XBlockWrapper(PageObject): grandkids.extend(descendant.children) grand_locators = [grandkid.locator for grandkid in grandkids] - return [descendant for descendant in descendants if not descendant.locator in grand_locators] + return [descendant for descendant in descendants if descendant.locator not in grand_locators] @property def preview_selector(self): diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 21be0b833b..073836db35 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -305,7 +305,7 @@ class CourseOutlineChild(PageObject, CourseOutlineItem): grandkids.extend(descendant.children) grand_locators = [grandkid.locator for grandkid in grandkids] - return [descendant for descendant in descendants if not descendant.locator in grand_locators] + return [descendant for descendant in descendants if descendant.locator not in grand_locators] class CourseOutlineUnit(CourseOutlineChild): diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index bf9ce68c58..79a2fc8471 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -324,29 +324,41 @@ class UnitPublishingTest(ContainerBase): ) ) ), - XBlockFixtureDesc('chapter', 'Unlocked Section', - metadata={'start': past_start_date.isoformat()}).add_children( - XBlockFixtureDesc('sequential', 'Unlocked Subsection').add_children( - XBlockFixtureDesc('vertical', 'Unlocked Unit').add_children( - XBlockFixtureDesc('problem', '', data=self.html_content) - ) - ) - ), - XBlockFixtureDesc('chapter', 'Section With Locked Unit').add_children( - XBlockFixtureDesc('sequential', 'Subsection With Locked Unit', - metadata={'start': past_start_date.isoformat()}).add_children( - XBlockFixtureDesc('vertical', 'Locked Unit', - metadata={'visible_to_staff_only': True}).add_children( - XBlockFixtureDesc('discussion', '', data=self.html_content) - ) - ) + XBlockFixtureDesc( + 'chapter', + 'Unlocked Section', + metadata={'start': past_start_date.isoformat()} + ).add_children( + XBlockFixtureDesc('sequential', 'Unlocked Subsection').add_children( + XBlockFixtureDesc('vertical', 'Unlocked Unit').add_children( + XBlockFixtureDesc('problem', '', data=self.html_content) + ) + ) ), - XBlockFixtureDesc('chapter', 'Unreleased Section', - metadata={'start': future_start_date.isoformat()}).add_children( - XBlockFixtureDesc('sequential', 'Unreleased Subsection').add_children( - XBlockFixtureDesc('vertical', 'Unreleased Unit') - ) - ) + XBlockFixtureDesc('chapter', 'Section With Locked Unit').add_children( + XBlockFixtureDesc( + 'sequential', + 'Subsection With Locked Unit', + metadata={'start': past_start_date.isoformat()} + ).add_children( + XBlockFixtureDesc( + 'vertical', + 'Locked Unit', + metadata={'visible_to_staff_only': True} + ).add_children( + XBlockFixtureDesc('discussion', '', data=self.html_content) + ) + ) + ), + XBlockFixtureDesc( + 'chapter', + 'Unreleased Section', + metadata={'start': future_start_date.isoformat()} + ).add_children( + XBlockFixtureDesc('sequential', 'Unreleased Subsection').add_children( + XBlockFixtureDesc('vertical', 'Unreleased Unit') + ) + ) ) def test_publishing(self): diff --git a/lms/djangoapps/certificates/management/commands/cert_whitelist.py b/lms/djangoapps/certificates/management/commands/cert_whitelist.py index 7604532dd0..1af6873250 100644 --- a/lms/djangoapps/certificates/management/commands/cert_whitelist.py +++ b/lms/djangoapps/certificates/management/commands/cert_whitelist.py @@ -2,6 +2,7 @@ Management command which sets or gets the certificate whitelist for a given user/course """ +from __future__ import print_function from django.core.management.base import BaseCommand, CommandError from optparse import make_option from opaque_keys import InvalidKeyError @@ -83,7 +84,8 @@ class Command(BaseCommand): cert_whitelist.save() whitelist = CertificateWhitelist.objects.filter(course_id=course) - print "User whitelist for course {0}:\n{1}".format(course_id, - '\n'.join(["{0} {1} {2}".format( - u.user.username, u.user.email, u.whitelist) - for u in whitelist])) + wl_users = '\n'.join( + "{u.user.username} {u.user.email} {u.whitelist}".format(u=u) + for u in whitelist + ) + print("User whitelist for course {0}:\n{1}".format(course_id, wl_users)) diff --git a/lms/djangoapps/courseware/features/problems_setup.py b/lms/djangoapps/courseware/features/problems_setup.py index eeaf7f6fee..e7ac96a535 100644 --- a/lms/djangoapps/courseware/features/problems_setup.py +++ b/lms/djangoapps/courseware/features/problems_setup.py @@ -361,7 +361,7 @@ def add_problem_to_course(course, problem_type, extra_meta=None): # Generate the problem XML using capa.tests.response_xml_factory factory_dict = PROBLEM_DICT[problem_type] problem_xml = factory_dict['factory'].build_xml(**factory_dict['kwargs']) - metadata = {'rerandomize': 'always'} if not 'metadata' in factory_dict else factory_dict['metadata'] + metadata = {'rerandomize': 'always'} if 'metadata' not in factory_dict else factory_dict['metadata'] if extra_meta: metadata = dict(metadata, **extra_meta) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 9134b14872..41169966e3 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -277,8 +277,9 @@ def _grade(student, request, course, keep_raw_scores): grade_summary['grade'] = letter_grade grade_summary['totaled_scores'] = totaled_scores # make this available, eg for instructor download & debugging if keep_raw_scores: - grade_summary['raw_scores'] = raw_scores # way to get all RAW scores out to instructor - # so grader can be double-checked + # way to get all RAW scores out to instructor + # so grader can be double-checked + grade_summary['raw_scores'] = raw_scores return grade_summary diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 23b56d1918..5173b427cb 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -349,8 +349,8 @@ class DjangoKeyValueStore(KeyValueStore): state[field.field_name] = kv_dict[field] field_object.state = json.dumps(state) else: - # The remaining scopes save fields on different rows, so - # we don't have to worry about conflicts + # The remaining scopes save fields on different rows, so + # we don't have to worry about conflicts field_object.value = json.dumps(kv_dict[field]) for field_object in field_objects: diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 904204764b..ae6a4b0dab 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -333,18 +333,20 @@ class TestCourseGrader(TestSubmittingProblems): """ grading_policy = { - "GRADER": [{ - "type": "Homework", - "min_count": 1, - "drop_count": 0, - "short_label": "HW", - "weight": 0.25 - }, { - "type": "Final", - "name": "Final Section", - "short_label": "Final", - "weight": 0.75 - }] + "GRADER": [ + { + "type": "Homework", + "min_count": 1, + "drop_count": 0, + "short_label": "HW", + "weight": 0.25 + }, { + "type": "Final", + "name": "Final Section", + "short_label": "Final", + "weight": 0.75 + } + ] } self.add_grading_policy(grading_policy) @@ -361,13 +363,14 @@ class TestCourseGrader(TestSubmittingProblems): grading_policy = { "GRADER": [ - { - "type": "Homework", - "min_count": 3, - "drop_count": 1, - "short_label": "HW", - "weight": 1 - }] + { + "type": "Homework", + "min_count": 3, + "drop_count": 1, + "short_label": "HW", + "weight": 1 + } + ] } self.add_grading_policy(grading_policy) diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index ca913d130e..704d68cd79 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -118,12 +118,8 @@ class TestVideo(BaseTestXmodule): for user in self.users } - self.assertEqual( - set([ - response.status_code - for _, response in responses.items() - ]).pop(), - 404) + status_codes = {response.status_code for response in responses.values()} + self.assertEqual(status_codes.pop(), 404) def test_handle_ajax(self): @@ -363,7 +359,7 @@ class TestTranscriptTranslationGetDispatch(TestVideo): u'end': [75], u'start': [9], u'text': [ - u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' + u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' ] } self.assertDictEqual(json.loads(response.body), calculated_0_75) @@ -375,7 +371,7 @@ class TestTranscriptTranslationGetDispatch(TestVideo): u'end': [150], u'start': [18], u'text': [ - u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' + u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' ] } self.assertDictEqual(json.loads(response.body), calculated_1_5) @@ -396,7 +392,7 @@ class TestTranscriptTranslationGetDispatch(TestVideo): u'end': [100], u'start': [12], u'text': [ - u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' + u'\u041f\u0440\u0438\u0432\u0456\u0442, edX \u0432\u0456\u0442\u0430\u0454 \u0432\u0430\u0441.' ] } self.non_en_file.seek(0) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 1721abd9b9..248a6593fd 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -226,12 +226,8 @@ class TestWordCloud(BaseTestXmodule): for user in self.users } - self.assertEqual( - set([ - response.status_code - for _, response in responses.items() - ]).pop(), - 200) + status_codes = {response.status_code for response in responses.values()} + self.assertEqual(status_codes.pop(), 200) for user in self.users: self.assertDictEqual( @@ -239,7 +235,8 @@ class TestWordCloud(BaseTestXmodule): { 'status': 'fail', 'error': 'Unknown Command!' - }) + } + ) def test_word_cloud_constructor(self): """Make sure that all parameters extracted correclty from xml""" diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index d6935d2254..2e730ec1ae 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -87,7 +87,7 @@ def switch_branch(branch, rdir): except subprocess.CalledProcessError as ex: log.exception('Getting a list of remote branches failed: %r', ex.output) raise GitImportError(GitImportError.CANNOT_BRANCH) - if not branch in output: + if branch not in output: raise GitImportError(GitImportError.REMOTE_BRANCH_MISSING) # Check it the remote branch has already been made locally cmd = ['git', 'branch', '-a', ] diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index dfb1879108..9a5693ea11 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -167,7 +167,7 @@ class Users(SysadminDashboardView): msg = u'' if settings.FEATURES['AUTH_USE_CERTIFICATES']: - if not '@' in uname: + if '@' not in uname: email = '{0}@{1}'.format(uname, email_domain) else: email = uname @@ -190,7 +190,7 @@ class Users(SysadminDashboardView): email = uname - if not '@' in email: + if '@' not in email: msg += _('email address required (not username)') return msg new_password = password diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index c3db473e63..2a158f84e1 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -112,8 +112,8 @@ def create_thread(request, course_id, commentable_id): thread.save() - #patch for backward compatibility to comments service - if not 'pinned' in thread.attributes: + # patch for backward compatibility to comments service + if 'pinned' not in thread.attributes: thread['pinned'] = False if post.get('auto_subscribe', 'false').lower() == 'true': diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 339a4112ef..96ccddfe52 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -119,8 +119,8 @@ def get_threads(request, course_key, discussion_id=None, per_page=THREADS_PER_PA threads, page, num_pages, corrected_text = cc.Thread.search(query_params) for thread in threads: - #patch for backward compatibility to comments service - if not 'pinned' in thread: + # patch for backward compatibility to comments service + if 'pinned' not in thread: thread['pinned'] = False query_params['page'] = page @@ -286,8 +286,8 @@ def single_thread(request, course_id, discussion_id, thread_id): add_courseware_context(threads, course) for thread in threads: - #patch for backward compatibility with comments service - if not "pinned" in thread: + # patch for backward compatibility with comments service + if "pinned" not in thread: thread["pinned"] = False threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index 30d2639144..1ee08bcca3 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -90,7 +90,7 @@ def _check_conditions_permissions(user, permissions, course_id, content): if operator == "or": return True in results elif operator == "and": - return not False in results + return False not in results return test(user, permissions, operator="or") diff --git a/lms/djangoapps/django_comment_client/tests/mock_cs_server/mock_cs_server.py b/lms/djangoapps/django_comment_client/tests/mock_cs_server/mock_cs_server.py index 974f0e276e..ec120885ec 100644 --- a/lms/djangoapps/django_comment_client/tests/mock_cs_server/mock_cs_server.py +++ b/lms/djangoapps/django_comment_client/tests/mock_cs_server/mock_cs_server.py @@ -23,8 +23,10 @@ class MockCommentServiceRequestHandler(BaseHTTPRequestHandler): post_dict = json.loads(data_string) # Log the request - logger.debug("Comment Service received POST request %s to path %s" % - (json.dumps(post_dict), self.path)) + logger.debug( + "Comment Service received POST request {0} to path {1}" + .format(json.dumps(post_dict), self.path) + ) # Every good post has at least an API key if 'X-Edx-Api-Key' in self.headers: @@ -58,8 +60,10 @@ class MockCommentServiceRequestHandler(BaseHTTPRequestHandler): post_dict = json.loads(data_string) # Log the request - logger.debug("Comment Service received PUT request %s to path %s" % - (json.dumps(post_dict), self.path)) + logger.debug( + "Comment Service received PUT request {0} to path {1}" + .format(json.dumps(post_dict), self.path) + ) # Every good post has at least an API key if 'X-Edx-Api-Key' in self.headers: diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 45fce6b7f7..f236925255 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -649,7 +649,7 @@ def modify_access(request, course_id): rolename = request.GET.get('rolename') action = request.GET.get('action') - if not rolename in ['instructor', 'staff', 'beta']: + if rolename not in ['instructor', 'staff', 'beta']: return HttpResponseBadRequest(strip_tags( "unknown rolename '{}'".format(rolename) )) @@ -712,7 +712,7 @@ def list_course_role_members(request, course_id): rolename = request.GET.get('rolename') - if not rolename in ['instructor', 'staff', 'beta']: + if rolename not in ['instructor', 'staff', 'beta']: return HttpResponseBadRequest() def extract_user_info(user): @@ -1336,7 +1336,7 @@ def get_distribution(request, course_id): available_features = instructor_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 feature not in available_features + (None,): return HttpResponseBadRequest(strip_tags( "feature '{}' not available.".format(feature) )) @@ -1349,7 +1349,7 @@ def get_distribution(request, course_id): } p_dist = None - if not feature is None: + if feature is not None: p_dist = instructor_analytics.distributions.profile_distribution(course_id, feature) response_payload['feature_results'] = { 'feature': p_dist.feature, @@ -1683,7 +1683,7 @@ def list_forum_members(request, course_id): return HttpResponseBadRequest("Operation requires instructor access.") # filter out unsupported for roles - if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: + if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: return HttpResponseBadRequest(strip_tags( "Unrecognized rolename '{}'.".format(rolename) )) @@ -1807,7 +1807,7 @@ def update_forum_role_membership(request, course_id): if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: return HttpResponseBadRequest("Operation requires instructor access.") - if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: + if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: return HttpResponseBadRequest(strip_tags( "Unrecognized rolename '{}'.".format(rolename) )) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index d6ccb40805..0ba515b7d4 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -87,7 +87,7 @@ def instructor_dashboard_2(request, course_id): if settings.FEATURES['CLASS_DASHBOARD'] and access['staff']: sections.append(_section_metrics(course, access)) - # Gate access to Ecommerce tab + # Gate access to Ecommerce tab if course_mode_has_price: sections.append(_section_e_commerce(course, access)) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index afcd5c8c4d..cc1b10bb12 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -339,7 +339,7 @@ def instructor_dashboard(request, course_id): msg2, datatable = _do_remote_gradebook(request.user, course, 'get-membership', dict(section=section)) msg += msg2 - if not 'List' in action: + if 'List' not in action: students = ','.join([x['email'] for x in datatable['retdata']]) overload = 'Overload' in action secure = request.is_secure() diff --git a/lms/djangoapps/instructor_analytics/distributions.py b/lms/djangoapps/instructor_analytics/distributions.py index 9a6b4fb568..423db91b93 100644 --- a/lms/djangoapps/instructor_analytics/distributions.py +++ b/lms/djangoapps/instructor_analytics/distributions.py @@ -92,7 +92,7 @@ def profile_distribution(course_id, feature): data types are EASY_CHOICE or OPEN_CHOICE """ - if not feature in AVAILABLE_PROFILE_FEATURES: + if feature not in AVAILABLE_PROFILE_FEATURES: raise ValueError( "unsupported feature requested for distribution '{}'".format( feature) diff --git a/lms/djangoapps/lms_migration/management/commands/create_user.py b/lms/djangoapps/lms_migration/management/commands/create_user.py index 20e5e045bf..55590f6b2b 100644 --- a/lms/djangoapps/lms_migration/management/commands/create_user.py +++ b/lms/djangoapps/lms_migration/management/commands/create_user.py @@ -148,7 +148,7 @@ class Command(BaseCommand): gname = raw_input("Add group (tab to autocomplete, empty line to end): ") if not gname: break - if not gname in groups: + if gname not in groups: print "Unknown group %s" % gname continue g = Group.objects.get(name=gname) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index b8930cfcdd..1fe0d82e39 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -332,8 +332,8 @@ def take_action_on_flags(request, course_id): except GradingServiceError: log.exception( u"Error taking action on flagged peer grading submissions, " - u"submission_id: {0}, action_type: {1}, grader_id: {2}".format( - submission_id, action_type, student_id) + u"submission_id: {0}, action_type: {1}, grader_id: {2}" + .format(submission_id, action_type, student_id) ) response = { 'success': False, diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 7c5851731e..56aa349e3c 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -930,9 +930,10 @@ class PaidCourseRegistration(OrderItem): Returns the order item """ - # First a bunch of sanity checks - course = modulestore().get_course(course_id) # actually fetch the course to make sure it exists, use this to - # throw errors if it doesn't + # First a bunch of sanity checks: + # actually fetch the course to make sure it exists, use this to + # throw errors if it doesn't. + course = modulestore().get_course(course_id) if not course: log.error("User {} tried to add non-existent course {} to cart id {}" .format(order.user.email, course_id, order.id)) @@ -1075,9 +1076,10 @@ class CourseRegCodeItem(OrderItem): Returns the order item """ - # First a bunch of sanity checks - course = modulestore().get_course(course_id) # actually fetch the course to make sure it exists, use this to - # throw errors if it doesn't + # First a bunch of sanity checks: + # actually fetch the course to make sure it exists, use this to + # throw errors if it doesn't. + course = modulestore().get_course(course_id) if not course: log.error("User {} tried to add non-existent course {} to cart id {}" .format(order.user.email, course_id, order.id)) diff --git a/lms/djangoapps/shoppingcart/tests/payment_fake.py b/lms/djangoapps/shoppingcart/tests/payment_fake.py index 3202b944f0..f2d2a6a2f6 100644 --- a/lms/djangoapps/shoppingcart/tests/payment_fake.py +++ b/lms/djangoapps/shoppingcart/tests/payment_fake.py @@ -78,7 +78,7 @@ class PaymentFakeView(View): """ new_status = request.body - if not new_status in ["success", "failure"]: + if new_status not in ["success", "failure"]: return HttpResponseBadRequest() else: diff --git a/lms/envs/edx4edx_aws.py b/lms/envs/edx4edx_aws.py index 4ff2133e7b..13a1e1b9f7 100644 --- a/lms/envs/edx4edx_aws.py +++ b/lms/envs/edx4edx_aws.py @@ -35,8 +35,8 @@ STATICFILES_DIRS = [ ("handouts", DATA_DIR / "handouts"), ("subs", DATA_DIR / "subs"), -# This is how you would use the textbook images locally -# ("book", ENV_ROOT / "book_images"), + # This is how you would use the textbook images locally + # ("book", ENV_ROOT / "book_images"), ] MAKO_TEMPLATES['course'] = [DATA_DIR, EDX4EDX_ROOT] diff --git a/lms/envs/load_test.py b/lms/envs/load_test.py index c21e226527..d050b8b32f 100644 --- a/lms/envs/load_test.py +++ b/lms/envs/load_test.py @@ -9,7 +9,7 @@ Settings for load testing. from .aws import * # Disable CSRF for load testing -EXCLUDE_CSRF = lambda elem: not elem in [ +EXCLUDE_CSRF = lambda elem: elem not in [ 'django.core.context_processors.csrf', 'django.middleware.csrf.CsrfViewMiddleware' ] diff --git a/lms/envs/test.py b/lms/envs/test.py index de74f29c0f..357289cc56 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -199,8 +199,9 @@ filterwarnings('ignore', message='No request passed to the backend, unable to ra # Ignore deprecation warnings (so we don't clutter Jenkins builds/production) # https://docs.python.org/2/library/warnings.html#the-warnings-filter -simplefilter('ignore') # Change to "default" to see the first instance of each hit - # or "error" to convert all into errors +simplefilter('ignore') +# Change to "default" to see the first instance of each hit +# or "error" to convert all into errors ######### Third-party auth ########## FEATURES['ENABLE_THIRD_PARTY_AUTH'] = True From 530c16326dfa4ff445ca30cb526e507d562a483e Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 11 Dec 2014 11:35:45 -0500 Subject: [PATCH 3/8] Ignore "deprecated form of raising exception" PEP8 warning We do this intentionally, to preserve tracebacks --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 3ef65d1ece..9ab5ec2f0a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,5 +16,5 @@ no-path-adjustment=1 #pdb=1 [pep8] -ignore=E501,E265 +ignore=E501,E265,W602 exclude=migrations From 4152f5e4e81785f42ef61333b267ebe93e915070 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 11 Dec 2014 13:09:23 -0500 Subject: [PATCH 4/8] Address pylint issue from diff-cover --- common/djangoapps/django_comment_common/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index feba43a0eb..03372a9422 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -67,6 +67,10 @@ class Role(models.Model): # TODO the name of this method is a little bit confusing, # since it's one-off and doesn't handle inheritance later def inherit_permissions(self, role): + """ + Make this role inherit permissions from the given role. + Permissions are only added, not removed. Does not handle inheritance. + """ if role.course_id and role.course_id != self.course_id: logging.warning( "%s cannot inherit permissions from %s due to course_id inconsistency", From 838ce74a3f22684370ddb4209449fb1c83b4fc93 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 12 Dec 2014 10:21:34 -0500 Subject: [PATCH 5/8] Add comments about disabled pep8 rules --- setup.cfg | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/setup.cfg b/setup.cfg index 9ab5ec2f0a..1acdbfe886 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,5 +16,17 @@ no-path-adjustment=1 #pdb=1 [pep8] +# error codes: http://pep8.readthedocs.org/en/latest/intro.html#error-codes +# E501: line too long +# E265: block comment should start with ‘# ‘ +# We have lots of comments that look like "##### HEADING #####" which violate +# this rule, because they don't have a space after the first #. However, +# they're still perfectly reasonable comments, so we disable this rule. +# W602: deprecated form of raising exception +# We do this in a few places to modify the exception message while preserving +# the traceback. See this blog post for more info: +# http://nedbatchelder.com/blog/200711/rethrowing_exceptions_in_python.html +# It's a little unusual, but we have good reasons for doing so, so we disable +# this rule. ignore=E501,E265,W602 exclude=migrations From 4fcf1f9eaed444149d8456ec9759f435541a488c Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 12 Dec 2014 10:47:58 -0500 Subject: [PATCH 6/8] Move simplefilter comments before call --- cms/envs/test.py | 2 +- lms/envs/test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/envs/test.py b/cms/envs/test.py index 9e3d760dff..2d16ded81a 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -158,9 +158,9 @@ filterwarnings('ignore', message='No request passed to the backend, unable to ra # Ignore deprecation warnings (so we don't clutter Jenkins builds/production) # https://docs.python.org/2/library/warnings.html#the-warnings-filter -simplefilter('ignore') # Change to "default" to see the first instance of each hit # or "error" to convert all into errors +simplefilter('ignore') ################################# CELERY ###################################### diff --git a/lms/envs/test.py b/lms/envs/test.py index 357289cc56..80206c1c91 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -199,9 +199,9 @@ filterwarnings('ignore', message='No request passed to the backend, unable to ra # Ignore deprecation warnings (so we don't clutter Jenkins builds/production) # https://docs.python.org/2/library/warnings.html#the-warnings-filter -simplefilter('ignore') # Change to "default" to see the first instance of each hit # or "error" to convert all into errors +simplefilter('ignore') ######### Third-party auth ########## FEATURES['ENABLE_THIRD_PARTY_AUTH'] = True From 08a8afa61950bd002b53aab9c2ef17683e56f1a6 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 12 Dec 2014 11:52:59 -0500 Subject: [PATCH 7/8] For methods on PasswordHistory, only calculate if necessary Check settings.FEATURES["ADVANCED_SECURITY"] first --- common/djangoapps/student/models.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index a85caeda08..01d48b8425 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -452,50 +452,60 @@ class PasswordHistory(models.Model): """ Returns whether the configuration which limits password reuse has been turned on """ + if not settings.FEATURES['ADVANCED_SECURITY']: + return False min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( 'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE', 0 ) - return settings.FEATURES['ADVANCED_SECURITY'] and min_diff_pw > 0 + return min_diff_pw > 0 @classmethod def is_staff_password_reuse_restricted(cls): """ Returns whether the configuration which limits password reuse has been turned on """ + if not settings.FEATURES['ADVANCED_SECURITY']: + return False min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( 'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE', 0 ) - return settings.FEATURES['ADVANCED_SECURITY'] and min_diff_pw > 0 + return min_diff_pw > 0 @classmethod def is_password_reset_frequency_restricted(cls): """ Returns whether the configuration which limits the password reset frequency has been turned on """ + if not settings.FEATURES['ADVANCED_SECURITY']: + return False min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( 'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS' ) - return settings.FEATURES['ADVANCED_SECURITY'] and min_days_between_reset + return min_days_between_reset @classmethod def is_staff_forced_password_reset_enabled(cls): """ Returns whether the configuration which forces password resets to occur has been turned on """ + if not settings.FEATURES['ADVANCED_SECURITY']: + return False min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( 'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS' ) - return settings.FEATURES['ADVANCED_SECURITY'] and min_days_between_reset + return min_days_between_reset @classmethod def is_student_forced_password_reset_enabled(cls): """ Returns whether the configuration which forces password resets to occur has been turned on """ + if not settings.FEATURES['ADVANCED_SECURITY']: + return False min_days_pw_reset = settings.ADVANCED_SECURITY_CONFIG.get( 'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS' ) - return settings.FEATURES['ADVANCED_SECURITY'] and min_days_pw_reset + return min_days_pw_reset @classmethod def should_user_reset_password_now(cls, user): From 48c349ffde901ce1525c859123cb2eb59482d063 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 12 Dec 2014 14:19:49 -0500 Subject: [PATCH 8/8] Re-add and clarify comment --- .../xmodule/modulestore/split_mongo/split_mongo_kvs.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index 8b90ee47f9..0cfa672143 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -76,6 +76,15 @@ class SplitMongoKVS(InheritanceKeyValueStore): # set the field self._fields[key.field_name] = value + # This function is currently incomplete: it doesn't handle side effects. + # To complete this function, here is some pseudocode for what should happen: + # + # if key.scope == Scope.children: + # remove inheritance from any exchildren + # add inheritance to any new children + # if key.scope == Scope.settings: + # if inheritable, push down to children + def delete(self, key): # handle any special cases if key.scope not in [Scope.children, Scope.settings, Scope.content]: