diff --git a/cms/djangoapps/contentstore/git_export_utils.py b/cms/djangoapps/contentstore/git_export_utils.py index 42698db5a425..53db8b404fc8 100644 --- a/cms/djangoapps/contentstore/git_export_utils.py +++ b/cms/djangoapps/contentstore/git_export_utils.py @@ -157,7 +157,9 @@ def export_to_git(course_id, repo, user='', rdir=None): ident = GIT_EXPORT_DEFAULT_IDENT time_stamp = timezone.now() cwd = os.path.abspath(rdirp) - commit_msg = 'Export from Studio at {1}'.format(user, time_stamp) + commit_msg = "Export from Studio at {time_stamp}".format( + time_stamp=time_stamp, + ) try: cmd_log(['git', 'config', 'user.email', ident['email']], cwd) cmd_log(['git', 'config', 'user.name', ident['name']], cwd) diff --git a/cms/djangoapps/contentstore/management/commands/create_course.py b/cms/djangoapps/contentstore/management/commands/create_course.py index 12c7054d2a3c..19d88a597e61 100644 --- a/cms/djangoapps/contentstore/management/commands/create_course.py +++ b/cms/djangoapps/contentstore/management/commands/create_course.py @@ -40,7 +40,12 @@ def parse_args(self, *args): try: user = user_from_str(args[1]) except User.DoesNotExist: - raise CommandError("No user {} found: expected args are ".format(args[1], self.args)) + raise CommandError( + "No user {user} found: expected args are {args}".format( + user=args[1], + args=self.args, + ), + ) org = args[2] course = args[3] diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 08dae9f20df9..42c75108b90e 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -37,7 +37,7 @@ def handle(self, *args, **options): self.stdout.write("Importing. Data_dir={data}, course_dirs={courses}\n".format( data=data_dir, courses=course_dirs, - dis=do_import_static)) + )) mstore = modulestore() course_items = import_from_xml( diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b804ed07cba3..dd52d63f6b73 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1728,8 +1728,10 @@ def test_rerun_with_permission_denied(self): def test_rerun_error(self): error_message = "Mock Error Message" with mock.patch( - 'xmodule.modulestore.mixed.MixedModuleStore.clone_course', - mock.Mock(side_effect=Exception(error_message)) + 'xmodule.modulestore.mixed.MixedModuleStore.clone_course', + mock.Mock( + side_effect=Exception(error_message), + ), ): source_course = CourseFactory.create() destination_course_key = self.post_rerun_request(source_course.id) diff --git a/cms/djangoapps/contentstore/tests/test_core_caching.py b/cms/djangoapps/contentstore/tests/test_core_caching.py index 7470ac1d9d83..1d8aeec15876 100644 --- a/cms/djangoapps/contentstore/tests/test_core_caching.py +++ b/cms/djangoapps/contentstore/tests/test_core_caching.py @@ -3,7 +3,7 @@ from django.test import TestCase -class Content: +class Content(object): def __init__(self, location, content): self.location = location self.content = content diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 43f4b6782f5c..f1bf828b9938 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -11,7 +11,7 @@ from django.test import RequestFactory from contentstore.views.course import _accessible_courses_list, _accessible_courses_list_from_groups, AccessListFallback -from contentstore.utils import delete_course_and_groups, reverse_course_url +from contentstore.utils import delete_course_and_groups from contentstore.tests.utils import AjaxEnabledTestClient from student.tests.factories import UserFactory from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff, OrgStaffRole, OrgInstructorRole diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py index 4dede3f71c2f..befcec8baba4 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -4,7 +4,6 @@ import json from contentstore.tests.utils import CourseTestCase from student.models import CourseEnrollment -from xmodule.modulestore.django import modulestore from contentstore.utils import reverse_course_url diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index bcb77fb4954c..65e86ddc4f48 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -6,8 +6,6 @@ import textwrap from mock import patch, Mock -from pymongo import MongoClient - from django.test.utils import override_settings from django.conf import settings from django.utils import translation diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 4c44f999b865..a9a0c4247409 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -16,7 +16,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.django import modulestore -from opaque_keys.edx.locator import CourseLocator class LMSLinksTestCase(TestCase): diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 49fab3360aba..fc367573d3de 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -7,7 +7,6 @@ from django.conf import settings from django.contrib.auth.models import User from django.test.client import Client -from django.test.utils import override_settings from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation from contentstore.utils import reverse_url @@ -17,7 +16,6 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.xml_importer import import_from_xml diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 4740a9391001..c2472c5c1294 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -331,7 +331,7 @@ def _update_asset(request, course_key, asset_key): contentstore().delete(thumbnail_content.get_id()) # remove from any caching del_cached_content(thumbnail_location) - except: + except Exception: logging.warning('Could not delete thumbnail: %s', thumbnail_location) # delete the original diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index ea4412e7b7d1..fd0a41120ed5 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -29,11 +29,12 @@ from django.utils.translation import ugettext as _ from models.settings.course_grading import CourseGradingModel -__all__ = ['OPEN_ENDED_COMPONENT_TYPES', - 'ADVANCED_COMPONENT_POLICY_KEY', - 'container_handler', - 'component_handler' - ] +__all__ = [ + 'OPEN_ENDED_COMPONENT_TYPES', + 'ADVANCED_COMPONENT_POLICY_KEY', + 'container_handler', + 'component_handler' +] log = logging.getLogger(__name__) @@ -331,7 +332,6 @@ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): "Advanced component %s does not exist. It will not be added to the Studio new component menu.", category ) - pass else: log.error( "Improper format for course advanced keys! %s", diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ef2c648f5930..fc9ec55d083b 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -653,8 +653,15 @@ def _create_or_rerun_course(request): 'course number so that it is unique.'), }) except InvalidKeyError as error: - return JsonResponse({ - "ErrMsg": _("Unable to create course '{name}'.\n\n{err}").format(name=display_name, err=error.message)} + return JsonResponse( + { + 'ErrMsg': _( + "Unable to create course '{name}'.\n\n{err}" + ).format( + name=display_name, + err=error.message, + ), + }, ) @@ -804,7 +811,7 @@ def course_info_update_handler(request, course_key_string, provided_id=None): elif request.method == 'DELETE': try: return JsonResponse(delete_course_update(usage_key, request.json, provided_id, request.user)) - except: + except Exception: return HttpResponseBadRequest( "Failed to delete", content_type="text/plain" @@ -813,7 +820,7 @@ def course_info_update_handler(request, course_key_string, provided_id=None): elif request.method in ('POST', 'PUT'): try: return JsonResponse(update_course_updates(usage_key, request.json, provided_id, request.user)) - except: + except Exception: return HttpResponseBadRequest( "Failed to save", content_type="text/plain" diff --git a/cms/djangoapps/contentstore/views/entrance_exam.py b/cms/djangoapps/contentstore/views/entrance_exam.py index 7c8658a4fe4a..bd725b839615 100644 --- a/cms/djangoapps/contentstore/views/entrance_exam.py +++ b/cms/djangoapps/contentstore/views/entrance_exam.py @@ -8,7 +8,6 @@ from django.contrib.auth.decorators import login_required from django_future.csrf import ensure_csrf_cookie from django.http import HttpResponse -from django.test import RequestFactory from contentstore.views.helpers import create_xblock from contentstore.views.item import delete_item diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 86cb005f17da..d290d26f612d 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -24,7 +24,7 @@ from xblock.fragment import Fragment import xmodule -from xmodule.tabs import StaticTab, CourseTabList +from xmodule.tabs import CourseTabList from xmodule.modulestore import ModuleStoreEnum, EdxJSONEncoder from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError @@ -47,7 +47,7 @@ from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url -from opaque_keys.edx.keys import UsageKey, CourseKey +from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryUsageLocator from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index c67d99d3bea1..107e5dc955f2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -6,7 +6,6 @@ from mock import Mock from xblock.core import XBlock -from django.test import TestCase from django.test.client import RequestFactory from xblock.core import XBlockAside diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index fd6e5aacac2a..bccc7a3da0d6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -3,7 +3,6 @@ import json from contentstore.views import tabs from contentstore.tests.utils import CourseTestCase -from django.test import TestCase from xmodule.x_module import STUDENT_VIEW from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index dbce92a02230..69d66fb28e5f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -6,7 +6,6 @@ from uuid import uuid4 import copy import textwrap -from pymongo import MongoClient from django.core.urlresolvers import reverse from django.test.utils import override_settings diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index e1232953e518..c391544e3100 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -319,9 +319,9 @@ def videos_post(course, request): edx_video_id = unicode(uuid4()) key = storage_service_key(bucket, file_name=edx_video_id) for metadata_name, value in [ - ("course_video_upload_token", course_video_upload_token), - ("client_video_id", file_name), - ("course_key", unicode(course.id)), + ('course_video_upload_token', course_video_upload_token), + ('client_video_id', file_name), + ('course_key', unicode(course.id)), ]: key.set_metadata(metadata_name, value) upload_url = key.generate_url( diff --git a/cms/djangoapps/contentstore/views/xblock.py b/cms/djangoapps/contentstore/views/xblock.py index f45df73fd33a..56a4289d5639 100644 --- a/cms/djangoapps/contentstore/views/xblock.py +++ b/cms/djangoapps/contentstore/views/xblock.py @@ -25,7 +25,7 @@ def xblock_resource(request, block_type, uri): # pylint: disable=unused-argumen except IOError: log.info('Failed to load xblock resource', exc_info=True) raise Http404 - except Exception: # pylint: disable-msg=broad-except + except Exception: # pylint: disable=broad-except log.error('Failed to load xblock resource', exc_info=True) raise Http404 diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 5eaa8c4ac36d..119926464d54 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -107,7 +107,7 @@ def send_user_notification_callback(sender, **kwargs): try: user.email_user(subject, message, studio_request_email) - except: + except Exception: log.warning("Unable to send course creator status e-mail to %s", user.email) diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index eb86f7e63603..207bcd2eb2f8 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -37,13 +37,14 @@ def fetch_grader(course_key, index): # return empty model else: - return {"id": index, - "type": "", - "min_count": 0, - "drop_count": 0, - "short_label": None, - "weight": 0 - } + return { + 'id': index, + 'type': '', + 'min_count': 0, + 'drop_count': 0, + 'short_label': None, + 'weight': 0, + } @staticmethod def update_from_json(course_key, jsondict, user): @@ -194,12 +195,13 @@ def convert_set_grace_period(descriptor): @staticmethod def parse_grader(json_grader): # manual to clear out kruft - result = {"type": json_grader["type"], - "min_count": int(json_grader.get('min_count', 0)), - "drop_count": int(json_grader.get('drop_count', 0)), - "short_label": json_grader.get('short_label', None), - "weight": float(json_grader.get('weight', 0)) / 100.0 - } + result = { + 'type': json_grader['type'], + 'min_count': int(json_grader.get('min_count', 0)), + 'drop_count': int(json_grader.get('drop_count', 0)), + 'short_label': json_grader.get('short_label', None), + 'weight': float(json_grader.get('weight', 0)) / 100.0, + } return result diff --git a/cms/envs/common.py b/cms/envs/common.py index fbc8eb311aa5..ca2a1c92ef19 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -136,9 +136,6 @@ # Prerequisite courses feature flag 'ENABLE_PREREQUISITE_COURSES': False, - # Toggle course milestones app/feature - 'MILESTONES_APP': False, - # Toggle course entrance exams feature 'ENTRANCE_EXAMS': False, diff --git a/cms/urls.py b/cms/urls.py index 4ca692f10975..e47a5d20ddaf 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -12,42 +12,111 @@ # Pattern to match a library key only LIBRARY_KEY_PATTERN = r'(?Plibrary-v1:[^/+]+\+[^/+]+)' -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', - url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'), - url(r'^transcripts/download$', 'contentstore.views.download_transcripts', name='download_transcripts'), - url(r'^transcripts/check$', 'contentstore.views.check_transcripts', name='check_transcripts'), - url(r'^transcripts/choose$', 'contentstore.views.choose_transcripts', name='choose_transcripts'), - url(r'^transcripts/replace$', 'contentstore.views.replace_transcripts', name='replace_transcripts'), - url(r'^transcripts/rename$', 'contentstore.views.rename_transcripts', name='rename_transcripts'), - url(r'^transcripts/save$', 'contentstore.views.save_transcripts', name='save_transcripts'), + url( + r'^transcripts/upload$', + 'contentstore.views.upload_transcripts', + name='upload_transcripts', + ), + url( + r'^transcripts/download$', + 'contentstore.views.download_transcripts', + name='download_transcripts', + ), + url( + r'^transcripts/check$', + 'contentstore.views.check_transcripts', + name='check_transcripts', + ), + url( + r'^transcripts/choose$', + 'contentstore.views.choose_transcripts', + name='choose_transcripts', + ), + url( + r'^transcripts/replace$', + 'contentstore.views.replace_transcripts', + name='replace_transcripts', + ), + url( + r'^transcripts/rename$', + 'contentstore.views.rename_transcripts', + name='rename_transcripts', + ), + url( + r'^transcripts/save$', + 'contentstore.views.save_transcripts', + name='save_transcripts', + ), - url(r'^preview/xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', - 'contentstore.views.preview_handler', name='preview_handler'), + url( + r'^preview/xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', + 'contentstore.views.preview_handler', + name='preview_handler', + ), - url(r'^xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', - 'contentstore.views.component_handler', name='component_handler'), + url( + r'^xblock/(?P.*?)/handler/(?P[^/]*)(?:/(?P.*))?$', + 'contentstore.views.component_handler', + name='component_handler', + ), - url(r'^xblock/resource/(?P[^/]*)/(?P.*)$', - 'contentstore.views.xblock.xblock_resource', name='xblock_resource_url'), + url( + r'^xblock/resource/(?P[^/]*)/(?P.*)$', + 'contentstore.views.xblock.xblock_resource', + name='xblock_resource_url', + ), # temporary landing page for a course - url(r'^edge/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', - 'contentstore.views.landing', name='landing'), + url( + r'^edge/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'contentstore.views.landing', + name='landing', + ), - url(r'^not_found$', 'contentstore.views.not_found', name='not_found'), - url(r'^server_error$', 'contentstore.views.server_error', name='server_error'), + url( + r'^not_found$', + 'contentstore.views.not_found', + name='not_found', + ), + url( + r'^server_error$', + 'contentstore.views.server_error', + name='server_error', + ), # temporary landing page for edge - url(r'^edge$', 'contentstore.views.edge', name='edge'), + url( + r'^edge$', + 'contentstore.views.edge', + name='edge', + ), # noop to squelch ajax errors - url(r'^event$', 'contentstore.views.event', name='event'), + url( + r'^event$', + 'contentstore.views.event', + name='event', + ), - url(r'^xmodule/', include('pipeline_js.urls')), - url(r'^heartbeat$', include('heartbeat.urls')), + url( + r'^xmodule/', + include('pipeline_js.urls'), + ), + url( + r'^heartbeat$', + include('heartbeat.urls'), + ), - url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')), - url(r'^lang_pref/', include('lang_pref.urls')), + url( + r'^user_api/', + include('openedx.core.djangoapps.user_api.urls'), + ), + url( + r'^lang_pref/', + include('lang_pref.urls'), + ), ) # User creation and updating views @@ -114,16 +183,20 @@ url(r'^api/val/v0/', include('edxval.urls')), ) -js_info_dict = { - 'domain': 'djangojs', - # We need to explicitly include external Django apps that are not in LOCALE_PATHS. - 'packages': ('openassessment',), -} - -urlpatterns += patterns( - '', +urlpatterns += ( # Serve catalog of localized strings to be rendered by Javascript - url(r'^i18n.js$', 'django.views.i18n.javascript_catalog', js_info_dict), + url( + r'^jsi18n/$', + 'django.views.i18n.javascript_catalog', + { + 'domain': 'djangojs', + # We need to explicitly include external Django apps that + # are not in LOCALE_PATHS. + 'packages': ( + 'openassessment', + ), + }, + ), ) if settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES'): diff --git a/common/__init__.py b/common/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 699db9aaf35e..54b849f74eb8 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -77,22 +77,22 @@ class LoncapaSystem(object): See :class:`ModuleSystem` for documentation of other attributes. """ - def __init__( # pylint: disable=invalid-name - self, - ajax_url, - anonymous_student_id, - cache, - can_execute_unsafe_code, - get_python_lib_zip, - DEBUG, # pylint: disable=invalid-name - filestore, - i18n, - node_path, - render_template, - seed, # Why do we do this if we have self.seed? - STATIC_URL, # pylint: disable=invalid-name - xqueue, - matlab_api_key=None + def __init__( # pylint: disable=invalid-name + self, + ajax_url, + anonymous_student_id, + cache, + can_execute_unsafe_code, + get_python_lib_zip, + DEBUG, # pylint: disable=invalid-name + filestore, + i18n, + node_path, + render_template, + seed, # Why do we do this if we have self.seed? + STATIC_URL, # pylint: disable=invalid-name + xqueue, + matlab_api_key=None, ): self.ajax_url = ajax_url self.anonymous_student_id = anonymous_student_id @@ -115,14 +115,14 @@ class LoncapaProblem(object): Main class for capa Problems. """ - def __init__(self, problem_text, id, capa_system, state=None, seed=None): + def __init__(self, problem_text, problem_id, capa_system, state=None, seed=None): """ Initializes capa Problem. Arguments: problem_text (string): xml defining the problem. - id (string): identifier for this problem, often a filename (no spaces). + problem_id (string): identifier for this problem, often a filename (no spaces). capa_system (LoncapaSystem): LoncapaSystem instance which provides OS, rendering, user context, and other resources. state (dict): containing the following keys: @@ -137,7 +137,7 @@ def __init__(self, problem_text, id, capa_system, state=None, seed=None): ## Initialize class variables from state self.do_reset() - self.problem_id = id + self.problem_id = problem_id self.capa_system = capa_system state = state or {} @@ -577,7 +577,11 @@ def _process_includes(self): parent = inc.getparent() parent.insert(parent.index(inc), incxml) parent.remove(inc) - log.debug('Included %s into %s' % (filename, self.problem_id)) + log.debug( + "Included %s into %s", + filename, + self.problem_id, + ) def _extract_system_path(self, script): """ @@ -596,20 +600,20 @@ def _extract_system_path(self, script): # find additional comma-separated modules search path path = [] - for dir in raw_path: - if not dir: + for directory in raw_path: + if not directory: continue - # path is an absolute path or a path relative to the data dir - dir = os.path.join(self.capa_system.filestore.root_path, dir) + # path is an absolute path or a path relative to the data directory + directory = os.path.join(self.capa_system.filestore.root_path, directory) # Check that we are within the filestore tree. - reldir = os.path.relpath(dir, self.capa_system.filestore.root_path) - if ".." in reldir: - log.warning("Ignoring Python directory outside of course: %r", dir) + directory_relative = os.path.relpath(directory, self.capa_system.filestore.root_path) + if '..' in directory_relative: + log.warning("Ignoring Python directory outside of course: %r", directory) continue - abs_dir = os.path.normpath(dir) - path.append(abs_dir) + directory_absolute = os.path.normpath(directory) + path.append(directory_absolute) return path diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index e470ac294e72..85daccda7270 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -38,15 +38,15 @@ def __iter__(self): # See the documentation for 'set_dict' for the use of kwargs def set( - self, - answer_id=None, - correctness=None, - npoints=None, - msg='', - hint='', - hintmode=None, - queuestate=None, - **kwargs + self, + answer_id=None, + correctness=None, + npoints=None, + msg='', + hint='', + hintmode=None, + queuestate=None, + **kwargs ): if answer_id is not None: @@ -125,15 +125,17 @@ def get_npoints(self, answer_id): # if not correct and no points have been assigned, return 0 return 0 - def set_property(self, answer_id, property, value): + def set_property(self, answer_id, key, value): if answer_id in self.cmap: - self.cmap[answer_id][property] = value + self.cmap[answer_id][key] = value else: - self.cmap[answer_id] = {property: value} + self.cmap[answer_id] = { + key: value, + } - def get_property(self, answer_id, property, default=None): + def get_property(self, answer_id, key, default=None): if answer_id in self.cmap: - return self.cmap[answer_id].get(property, default) + return self.cmap[answer_id].get(key, default) return default def get_correctness(self, answer_id): diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 9b7d3af9c95d..bd3847d7bd2d 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -868,8 +868,13 @@ def ungraded_response(self, queue_msg, queuekey): nothing """ # check the queuekey against the saved queuekey - if('queuestate' in self.input_state and self.input_state['queuestate'] == 'queued' - and self.input_state['queuekey'] == queuekey): + if ( + 'queuestate' in self.input_state + and + self.input_state['queuestate'] == 'queued' + and + self.input_state['queuekey'] == queuekey + ): msg = self._parse_data(queue_msg) # save the queue message so that it can be rendered later self.input_state['queue_msg'] = msg @@ -1013,8 +1018,6 @@ def get_attributes(cls): ] def _extra_context(self): - """ - """ context = { 'setup_script': '{static_url}js/capa/schematicinput.js'.format( static_url=self.capa_system.STATIC_URL), @@ -1087,9 +1090,10 @@ def get_attributes(cls): """ Note: height, width are required. """ - return [Attribute('height'), - Attribute('width'), - ] + return [ + Attribute('height'), + Attribute('width'), + ] # ------------------------------------------------------------------------- @@ -1109,11 +1113,12 @@ def get_attributes(cls): """ Note: height, width, molecules and geometries are required. """ - return [Attribute('height'), - Attribute('width'), - Attribute('molecules'), - Attribute('geometries'), - ] + return [ + Attribute('height'), + Attribute('width'), + Attribute('molecules'), + Attribute('geometries'), + ] #------------------------------------------------------------------------- @@ -1407,8 +1412,6 @@ def get_attributes(cls): Attribute('missing', None)] def _extra_context(self): - """ - """ context = { 'applet_loader': '{static_url}js/capa/editamolecule.js'.format( static_url=self.capa_system.STATIC_URL), @@ -1437,14 +1440,13 @@ def get_attributes(cls): """ Note: width, hight, and target_shape are required. """ - return [Attribute('width'), - Attribute('height'), - Attribute('target_shape') - ] + return [ + Attribute('width'), + Attribute('height'), + Attribute('target_shape'), + ] def _extra_context(self): - """ - """ context = { 'applet_loader': '{static_url}js/capa/design-protein-2d.js'.format( static_url=self.capa_system.STATIC_URL), @@ -1474,13 +1476,12 @@ def get_attributes(cls): """ Note: width, height, and dna_sequencee are required. """ - return [Attribute('genex_dna_sequence'), - Attribute('genex_problem_number') - ] + return [ + Attribute('genex_dna_sequence'), + Attribute('genex_problem_number'), + ] def _extra_context(self): - """ - """ context = { 'applet_loader': '{static_url}js/capa/edit-a-gene.js'.format( static_url=self.capa_system.STATIC_URL), @@ -1545,11 +1546,14 @@ def setup(self): def _find_options(self): """ Returns an array of dicts where each dict represents an option. """ elements = self.xml.findall('./options/option') - return [{ + return [ + { 'id': index, 'description': option.text, 'choice': option.get('choice') - } for (index, option) in enumerate(elements)] + } + for (index, option) in enumerate(elements) + ] def _validate_options(self): """ Raises a ValueError if the choice attribute is missing or invalid. """ diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 63eff0bc218e..b9354486eda4 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -338,9 +338,13 @@ def get_hints(self, student_answers, new_cmap, old_cmap): # # - if (self.hint_tag is not None - and hintgroup.find(self.hint_tag) is not None - and hasattr(self, 'check_hint_condition')): + if ( + self.hint_tag is not None + and + hintgroup.find(self.hint_tag) is not None + and + hasattr(self, 'check_hint_condition') + ): rephints = hintgroup.findall(self.hint_tag) hints_to_show = self.check_hint_condition( @@ -408,7 +412,7 @@ def _render_response_msg_html(self, response_msg): # If we can't do that, create the
and set the message # as the text of the
- except: + except Exception: response_msg_div = etree.Element('div') response_msg_div.text = str(response_msg) @@ -2803,11 +2807,14 @@ def _get_max_points(self): def _find_options(self, inputfield): """Returns an array of dicts where each dict represents an option. """ elements = inputfield.findall('./options/option') - return [{ + return [ + { 'id': index, 'description': option.text, 'choice': option.get('choice') - } for (index, option) in enumerate(elements)] + } + for (index, option) in enumerate(elements) + ] def _find_option_with_choice(self, inputfield, choice): """Returns the option with the given choice value, otherwise None. """ @@ -2854,10 +2861,11 @@ class ChoiceTextResponse(LoncapaResponse): human_name = _('Checkboxes With Text Input') tags = ['choicetextresponse'] max_inputfields = 1 - allowed_inputfields = ['choicetextgroup', - 'checkboxtextgroup', - 'radiotextgroup' - ] + allowed_inputfields = [ + 'choicetextgroup', + 'checkboxtextgroup', + 'radiotextgroup', + ] def __init__(self, *args, **kwargs): self.correct_inputs = {} @@ -2963,7 +2971,10 @@ def assign_choice_names(self): """ for index, choice in enumerate( - self.xml.xpath('//*[@id=$id]//choice', id=self.xml.get('id')) + self.xml.xpath( + '//*[@id=$id]//choice', + id=self.xml.get('id'), + ) ): # Set the name attribute for # "bc" is appended at the end to indicate that this is a diff --git a/common/lib/capa/capa/safe_exec/lazymod.py b/common/lib/capa/capa/safe_exec/lazymod.py index d8d6115ca30a..c43c97a55f02 100644 --- a/common/lib/capa/capa/safe_exec/lazymod.py +++ b/common/lib/capa/capa/safe_exec/lazymod.py @@ -39,5 +39,5 @@ def __getattr__(self, name): submod = getattr(mod, name) except ImportError: raise AttributeError("'module' object has no attribute %r" % name) - self.__dict__[name] = LazyModule(subname, submod) + self.__dict__[name] = LazyModule(subname) return self.__dict__[name] diff --git a/common/lib/capa/capa/safe_exec/safe_exec.py b/common/lib/capa/capa/safe_exec/safe_exec.py index b57afbcd0d46..1e9bd486cdd6 100644 --- a/common/lib/capa/capa/safe_exec/safe_exec.py +++ b/common/lib/capa/capa/safe_exec/safe_exec.py @@ -72,14 +72,14 @@ def update_hash(hasher, obj): @dog_stats_api.timed('capa.safe_exec.time') def safe_exec( - code, - globals_dict, - random_seed=None, - python_path=None, - extra_files=None, - cache=None, - slug=None, - unsafely=False, + code, + globals_dict, + random_seed=None, + python_path=None, + extra_files=None, + cache=None, + slug=None, + unsafely=False, ): """ Execute python code safely. diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index f1b524933877..ca39f3b5ff1b 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -27,8 +27,6 @@ def test_blank_problem(self): # Render the HTML etree.XML(problem.get_html()) - # expect that we made it here without blowing up - self.assertTrue(True) def test_include_html(self): # Create a test file to include diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 8f4693073209..0efb920058cd 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -62,8 +62,12 @@ def render_to_xml(self, context_dict): try: xml = etree.fromstring("" + xml_str + "") except Exception as exc: - raise TemplateError("Could not parse XML from '{0}': {1}".format( - xml_str, str(exc))) + raise TemplateError( + "Could not parse XML from '{0}': {1}".format( + xml_str, + str(exc), + ), + ) else: return xml diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index b1f42b659ec6..569982138a0f 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -72,10 +72,10 @@ def test_rendering(self): def test_option_parsing(self): f = inputtypes.OptionInput.parse_options - def check(input, options): + def check(input_value, options): """Take list of options, confirm that output is in the silly doubled format""" expected = [(o, o) for o in options] - self.assertEqual(f(input), expected) + self.assertEqual(f(input_value), expected) check("('a','b')", ['a', 'b']) check("('a', 'b')", ['a', 'b']) @@ -157,11 +157,21 @@ def test_rendering(self): display_class = "a_class" display_file = "my_files/hi.js" - xml_str = """""".format( + xml_str = ( + """ + + """ + ).format( params=params, ps=quote_attr(problem_state), - dc=display_class, df=display_file) + dc=display_class, + df=display_file, + ) element = etree.fromstring(xml_str) @@ -478,10 +488,12 @@ def test_rendering_with_state(self): def test_rendering_when_completed(self): for status in ['correct', 'incorrect']: - state = {'value': 'print "good evening"', - 'status': status, - 'input_state': {}, - } + state = { + 'value': 'print "good evening"', + 'status': status, + 'input_state': { + }, + } elt = etree.fromstring(self.xml) the_input = self.input_class(test_capa_system(), elt, state) @@ -509,10 +521,14 @@ def test_rendering_when_completed(self): @patch('capa.inputtypes.time.time', return_value=10) def test_rendering_while_queued(self, time): - state = {'value': 'print "good evening"', - 'status': 'incomplete', - 'input_state': {'queuestate': 'queued', 'queuetime': 5}, - } + state = { + 'value': 'print "good evening"', + 'status': 'incomplete', + 'input_state': { + 'queuestate': 'queued', + 'queuetime': 5, + }, + } elt = etree.fromstring(self.xml) the_input = self.input_class(test_capa_system(), elt, state) @@ -1214,22 +1230,83 @@ def test_rendering(self): 'status': 'unsubmitted'} user_input = { # order matters, for string comparison - "target_outline": "false", - "base_image": "/dummy-static/images/about_1.png", - "draggables": [ - {"can_reuse": "", "label": "Label 1", "id": "1", "icon": "", "target_fields": []}, - {"can_reuse": "", "label": "cc", "id": "name_with_icon", "icon": "/dummy-static/images/cc.jpg", "target_fields": []}, - {"can_reuse": "", "label": "arrow-left", "id": "with_icon", "icon": "/dummy-static/images/arrow-left.png", "target_fields": []}, - {"can_reuse": "", "label": "Label2", "id": "5", "icon": "", "target_fields": []}, - {"can_reuse": "", "label": "Mute", "id": "2", "icon": "/dummy-static/images/mute.png", "target_fields": []}, - {"can_reuse": "", "label": "spinner", "id": "name_label_icon3", "icon": "/dummy-static/images/spinner.gif", "target_fields": []}, - {"can_reuse": "", "label": "Star", "id": "name4", "icon": "/dummy-static/images/volume.png", "target_fields": []}, - {"can_reuse": "", "label": "Label3", "id": "7", "icon": "", "target_fields": []}], - "one_per_target": "True", - "targets": [ - {"y": "90", "x": "210", "id": "t1", "w": "90", "h": "90"}, - {"y": "160", "x": "370", "id": "t2", "w": "90", "h": "90"} - ] + 'target_outline': 'false', + 'base_image': '/dummy-static/images/about_1.png', + 'draggables': [ + { + 'can_reuse': '', + 'label': 'Label 1', + 'id': '1', + 'icon': '', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'cc', + 'id': 'name_with_icon', + 'icon': '/dummy-static/images/cc.jpg', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'arrow-left', + 'id': 'with_icon', + 'icon': '/dummy-static/images/arrow-left.png', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'Label2', + 'id': '5', + 'icon': '', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'Mute', + 'id': '2', + 'icon': '/dummy-static/images/mute.png', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'spinner', + 'id': 'name_label_icon3', + 'icon': '/dummy-static/images/spinner.gif', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'Star', + 'id': 'name4', + 'icon': '/dummy-static/images/volume.png', + 'target_fields': [], + }, + { + 'can_reuse': '', + 'label': 'Label3', + 'id': '7', + 'icon': '', + 'target_fields': [], + }, + ], + 'one_per_target': 'True', + 'targets': [ + { + 'y': '90', + 'x': '210', + 'id': 't1', + 'w': '90', + 'h': '90', + }, + { + 'y': '160', + 'x': '370', + 'id': 't2', + 'w': '90', + 'h': '90', + }, + ], } the_input = lookup_tag('drag_and_drop_input')(test_capa_system(), element, state) diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 5883af607461..0a61375605e4 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -23,8 +23,24 @@ from capa.responsetypes import LoncapaProblemError, \ StudentInputError, ResponseError from capa.correctmap import CorrectMap +from capa.tests.response_xml_factory import ( + AnnotationResponseXMLFactory, + ChoiceResponseXMLFactory, + CodeResponseXMLFactory, + ChoiceTextResponseXMLFactory, + CustomResponseXMLFactory, + FormulaResponseXMLFactory, + ImageResponseXMLFactory, + JavascriptResponseXMLFactory, + MultipleChoiceResponseXMLFactory, + NumericalResponseXMLFactory, + OptionResponseXMLFactory, + SchematicResponseXMLFactory, + StringResponseXMLFactory, + SymbolicResponseXMLFactory, + TrueFalseResponseXMLFactory, +) from capa.util import convert_files_to_filenames -from capa.util import compare_with_tolerance from capa.xqueue_interface import dateformat @@ -77,7 +93,6 @@ def _get_random_number_result(self, seed_value): class MultiChoiceResponseTest(ResponseTest): - from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory xml_factory_class = MultipleChoiceResponseXMLFactory def test_multiple_choice_grade(self): @@ -99,7 +114,6 @@ def test_named_multiple_choice_grade(self): class TrueFalseResponseTest(ResponseTest): - from capa.tests.response_xml_factory import TrueFalseResponseXMLFactory xml_factory_class = TrueFalseResponseXMLFactory def test_true_false_grade(self): @@ -139,7 +153,6 @@ def test_named_true_false_grade(self): class ImageResponseTest(ResponseTest): - from capa.tests.response_xml_factory import ImageResponseXMLFactory xml_factory_class = ImageResponseXMLFactory def test_rectangle_grade(self): @@ -203,7 +216,6 @@ def test_show_answer(self): class SymbolicResponseTest(ResponseTest): - from capa.tests.response_xml_factory import SymbolicResponseXMLFactory xml_factory_class = SymbolicResponseXMLFactory def test_grade_single_input_correct(self): @@ -294,8 +306,12 @@ def test_multiple_inputs_exception(self): self.build_problem(math_display=True, expect="2*x+3*y", num_inputs=3) def _assert_symbolic_grade( - self, problem, student_input, dynamath_input, expected_correctness, - snuggletex_resp="" + self, + problem, + student_input, + dynamath_input, + expected_correctness, + snuggletex_resp='', ): """ Assert that the symbolic response has a certain grade. @@ -321,7 +337,6 @@ def _assert_symbolic_grade( class OptionResponseTest(ResponseTest): - from capa.tests.response_xml_factory import OptionResponseXMLFactory xml_factory_class = OptionResponseXMLFactory def test_grade(self): @@ -352,7 +367,6 @@ class FormulaResponseTest(ResponseTest): """ Test the FormulaResponse class """ - from capa.tests.response_xml_factory import FormulaResponseXMLFactory xml_factory_class = FormulaResponseXMLFactory def test_grade(self): @@ -501,7 +515,6 @@ def test_validate_answer(self): class StringResponseTest(ResponseTest): - from capa.tests.response_xml_factory import StringResponseXMLFactory xml_factory_class = StringResponseXMLFactory def test_backward_compatibility_for_multiple_answers(self): @@ -851,7 +864,6 @@ def gimme_a_random_hint(answer_ids, student_answers, new_cmap, old_cmap): class CodeResponseTest(ResponseTest): - from capa.tests.response_xml_factory import CodeResponseXMLFactory xml_factory_class = CodeResponseXMLFactory def setUp(self): @@ -1043,7 +1055,6 @@ def test_parse_score_msg_of_responder(self): class ChoiceResponseTest(ResponseTest): - from capa.tests.response_xml_factory import ChoiceResponseXMLFactory xml_factory_class = ChoiceResponseXMLFactory def test_radio_group_grade(self): @@ -1086,7 +1097,6 @@ def test_grade_with_no_checkbox_selected(self): class JavascriptResponseTest(ResponseTest): - from capa.tests.response_xml_factory import JavascriptResponseXMLFactory xml_factory_class = JavascriptResponseXMLFactory def test_grade(self): @@ -1127,7 +1137,6 @@ def test_cant_execute_javascript(self): class NumericalResponseTest(ResponseTest): - from capa.tests.response_xml_factory import NumericalResponseXMLFactory xml_factory_class = NumericalResponseXMLFactory # We blend the line between integration (using evaluator) and exclusively @@ -1352,7 +1361,6 @@ def test_validate_answer(self): class CustomResponseTest(ResponseTest): - from capa.tests.response_xml_factory import CustomResponseXMLFactory xml_factory_class = CustomResponseXMLFactory def test_inline_code(self): @@ -1828,7 +1836,7 @@ def seventeen(): num = my_helper.seventeen() """) capa_system = test_capa_system() - capa_system.get_python_lib_zip = lambda: zipstring.getvalue() + capa_system.get_python_lib_zip = zipstring.getvalue() problem = self.build_problem(script=script, capa_system=capa_system) self.assertEqual(problem.context['num'], 17) @@ -1904,7 +1912,6 @@ class SchematicResponseTest(ResponseTest): """ Class containing setup and tests for Schematic responsetype. """ - from capa.tests.response_xml_factory import SchematicResponseXMLFactory xml_factory_class = SchematicResponseXMLFactory def test_grade(self): @@ -1955,7 +1962,6 @@ def test_script_exception(self): class AnnotationResponseTest(ResponseTest): - from capa.tests.response_xml_factory import AnnotationResponseXMLFactory xml_factory_class = AnnotationResponseXMLFactory def test_grade(self): @@ -1997,7 +2003,6 @@ class ChoiceTextResponseTest(ResponseTest): Class containing setup and tests for ChoiceText responsetype. """ - from response_xml_factory import ChoiceTextResponseXMLFactory xml_factory_class = ChoiceTextResponseXMLFactory # `TEST_INPUTS` is a dictionary mapping from @@ -2173,7 +2178,6 @@ def test_valid_xml(self): Test that `build_problem` builds valid xml """ self.build_problem() - self.assertTrue(True) def test_unchecked_input_not_validated(self): """ @@ -2227,10 +2231,23 @@ def test_interpret_error(self): def test_staff_answer_error(self): broken_problem = self._make_problem( - [("true", {"answer": "Platypus", "tolerance": "0"}), - ("true", {"answer": "edX", "tolerance": "0"}) - ], - "checkboxtextgroup" + [ + ( + 'true', + { + 'answer': 'Platypus', + 'tolerance': '0', + }, + ), + ( + 'true', + { + 'answer': 'edX', + 'tolerance': '0', + }, + ), + ], + 'checkboxtextgroup', ) with self.assertRaisesRegexp( StudentInputError, @@ -2318,10 +2335,23 @@ def test_checkbox_grades(self): ) # Two choice two input problem with both choices correct. checkbox_two_choices_two_inputs = self._make_problem( - [("true", {"answer": "123", "tolerance": "0"}), - ("true", {"answer": "456", "tolerance": "0"}) - ], - "checkboxtextgroup" + [ + ( + 'true', + { + 'answer': '123', + 'tolerance': '0', + }, + ), + ( + 'true', + { + 'answer': '456', + 'tolerance': '0', + }, + ), + ], + 'checkboxtextgroup', ) # Dictionary problem_name: problem diff --git a/common/lib/capa/capa/tests/test_shuffle.py b/common/lib/capa/capa/tests/test_shuffle.py index 958c0c23980c..d1cbfc31c310 100644 --- a/common/lib/capa/capa/tests/test_shuffle.py +++ b/common/lib/capa/capa/tests/test_shuffle.py @@ -274,8 +274,13 @@ def test_multiple_shuffle_responses(self): self.assertEqual(orig_html, problem.get_html(), 'should be able to call get_html() twice') html = orig_html.replace('\n', ' ') # avoid headaches with .* matching print html - self.assertRegexpMatches(html, r"
.*\[.*'Banana'.*'Apple'.*'Chocolate'.*'Donut'.*\].*
.*" + - r"
.*\[.*'C'.*'A'.*'D'.*'B'.*\].*
") + self.assertRegexpMatches( + html, + ( + r"
.*\[.*'Banana'.*'Apple'.*'Chocolate'.*'Donut'.*\].*
.*" + r"
.*\[.*'C'.*'A'.*'D'.*'B'.*\].*
" + ), + ) # Look at the responses in their authored order responses = sorted(problem.responders.values(), key=lambda resp: int(resp.id[resp.id.rindex('_') + 1:])) self.assertFalse(responses[0].has_mask()) diff --git a/common/lib/capa/capa/tests/test_util.py b/common/lib/capa/capa/tests/test_util.py index f6b76e1c9e70..9f19c7f615b6 100644 --- a/common/lib/capa/capa/tests/test_util.py +++ b/common/lib/capa/capa/tests/test_util.py @@ -2,7 +2,6 @@ Tests capa util """ import unittest -import textwrap from . import test_capa_system from capa.util import compare_with_tolerance, sanitize_html diff --git a/common/lib/chem/chem/chemcalc.py b/common/lib/chem/chem/chemcalc.py index 119b558fe46e..02373c68522b 100644 --- a/common/lib/chem/chem/chemcalc.py +++ b/common/lib/chem/chem/chemcalc.py @@ -339,12 +339,10 @@ def divide_chemical_expression(s1, s2, ignore_state=False): if treedic['1 phases'] != treedic['2 phases']: return False - if any( - [ + if any([ x / y - treedic['1 factors'][0] / treedic['2 factors'][0] for (x, y) in zip(treedic['1 factors'], treedic['2 factors']) - ] - ): + ]): # factors are not proportional return False else: diff --git a/common/lib/chem/chem/miller.py b/common/lib/chem/chem/miller.py index 3be90117e9a5..ad34317f689f 100644 --- a/common/lib/chem/chem/miller.py +++ b/common/lib/chem/chem/miller.py @@ -172,8 +172,14 @@ def miller(points): Y = np.array([new_origin[0], 1 - new_origin[1], new_origin[2]]) Z = np.array([new_origin[0], new_origin[1], 1 - new_origin[2]]) new_Ccs = [X - new_origin, Y - new_origin, Z - new_origin] - segments = ([np.dot(P - new_origin, N) / np.dot(ort, N) if - np.dot(ort, N) != 0 else np.nan for ort in new_Ccs]) + segments = [ + ( + np.dot(P - new_origin, N) / np.dot(ort, N) + if np.dot(ort, N) != 0 + else np.nan + ) + for ort in new_Ccs + ] # fix signs of indices: 0 -> 1, 1 -> -1 ( segments = (1 - 2 * new_origin) * segments diff --git a/common/lib/symmath/symmath/formula.py b/common/lib/symmath/symmath/formula.py index 0c6aa05a246d..b9d249e0be4d 100644 --- a/common/lib/symmath/symmath/formula.py +++ b/common/lib/symmath/symmath/formula.py @@ -99,11 +99,11 @@ def my_evalf(expr, chop=False): if type(expr) == list: try: return [x.evalf(chop=chop) for x in expr] - except: + except Exception: return expr try: return expr.evalf(chop=chop) - except: + except Exception: return expr @@ -115,23 +115,25 @@ def my_sympify(expr, normphase=False, matrix=False, abcsym=False, do_qubit=False if symtab: varset = symtab else: - varset = {'p': sympy.Symbol('p'), - 'g': sympy.Symbol('g'), - 'e': sympy.E, # for exp - 'i': sympy.I, # lowercase i is also sqrt(-1) - 'Q': sympy.Symbol('Q'), # otherwise it is a sympy "ask key" - 'I': sympy.Symbol('I'), # otherwise it is sqrt(-1) - 'N': sympy.Symbol('N'), # or it is some kind of sympy function - 'ZZ': sympy.Symbol('ZZ'), # otherwise it is the PythonIntegerRing - 'XI': sympy.Symbol('XI'), # otherwise it is the capital \XI - 'hat': sympy.Function('hat'), # for unit vectors (8.02) - } + varset = { + 'p': sympy.Symbol('p'), + 'g': sympy.Symbol('g'), + 'e': sympy.E, # for exp + 'i': sympy.I, # lowercase i is also sqrt(-1) + 'Q': sympy.Symbol('Q'), # otherwise it is a sympy "ask key" + 'I': sympy.Symbol('I'), # otherwise it is sqrt(-1) + 'N': sympy.Symbol('N'), # or it is some kind of sympy function + 'ZZ': sympy.Symbol('ZZ'), # otherwise it is the PythonIntegerRing + 'XI': sympy.Symbol('XI'), # otherwise it is the capital \XI + 'hat': sympy.Function('hat'), # for unit vectors (8.02) + } if do_qubit: # turn qubit(...) into Qubit instance - varset.update({'qubit': Qubit, - 'Ket': Ket, - 'dot': dot, - 'bit': sympy.Function('bit'), - }) + varset.update({ + 'qubit': Qubit, + 'Ket': Ket, + 'dot': dot, + 'bit': sympy.Function('bit'), + }) if abcsym: # consider all lowercase letters as real symbols, in the parsing for letter in string.lowercase: if letter in varset: # exclude those already done diff --git a/common/lib/symmath/symmath/symmath_check.py b/common/lib/symmath/symmath/symmath_check.py index e38da60b724e..26f42d1232f9 100644 --- a/common/lib/symmath/symmath/symmath_check.py +++ b/common/lib/symmath/symmath/symmath_check.py @@ -20,12 +20,13 @@ # This is one of the main entry points to call. -def symmath_check_simple(expect, ans, adict={}, symtab=None, extra_options=None): +def symmath_check_simple(expect, ans, adict=None, symtab=None, extra_options=None): """ Check a symbolic mathematical expression using sympy. The input is an ascii string (not MathML) converted to math using sympy.sympify. """ + adict = adict or {} options = {'__MATRIX__': False, '__ABC__': False, '__LOWER__': False} if extra_options: options.update(extra_options) @@ -40,15 +41,18 @@ def symmath_check_simple(expect, ans, adict={}, symtab=None, extra_options=None) ans = ans.lower() try: - ret = check(expect, ans, - matrix=options['__MATRIX__'], - abcsym=options['__ABC__'], - symtab=symtab, - ) + ret = check( + expect, + ans, + matrix=options['__MATRIX__'], + abcsym=options['__ABC__'], + symtab=symtab, + ) except Exception, err: - return {'ok': False, - 'msg': 'Error %s
Failed in evaluating check(%s,%s)' % (err, expect, ans) - } + return { + 'ok': False, + 'msg': 'Error %s
Failed in evaluating check(%s,%s)' % (err, expect, ans), + } return ret #----------------------------------------------------------------------------- @@ -254,7 +258,10 @@ def symmath_check(expect, ans, dynamath=None, options=None, debug=None, xml=None fsym = f.sympy msg += '

You entered: %s

' % to_latex(f.sympy) except Exception, err: - log.exception("Error evaluating expression '%s' as a valid equation" % ans) + log.exception( + "Error evaluating expression '%s' as a valid equation", + ans, + ) msg += "

Error in evaluating your expression '%s' as a valid equation

" % (ans) if "Illegal math" in str(err): msg += "

Illegal math expression

" diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 2a7bc5f0cbb5..1cd63057e571 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -113,7 +113,7 @@ def definition_from_xml(cls, xml_object, system): try: child_block = system.process_xml(etree.tostring(child)) child_content_urls.append(child_block.scope_ids.usage_id) - except: + except Exception: log.exception("Unable to load child when parsing ABTest. Continuing...") continue diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index 67ab204cf247..3f171442d8e6 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -22,14 +22,14 @@ def from_xml(cls, xml_data, system, id_generator): next_include = xml_object.find('include') while next_include is not None: system.error_tracker("WARNING: the tag is deprecated, and will go away.") - file = next_include.get('file') + path_file = next_include.get('file') parent = next_include.getparent() - if file is None: + if path_file is None: continue try: - ifp = system.resources_fs.open(file) + ifp = system.resources_fs.open(path_file) # read in and convert to XML incxml = etree.XML(ifp.read()) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index a3fca52714bf..6ef2768a8431 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -17,14 +17,12 @@ # pylint: disable=invalid-name dog_stats_api = None -from pkg_resources import resource_string - from capa.capa_problem import LoncapaProblem, LoncapaSystem from capa.responsetypes import StudentInputError, \ ResponseError, LoncapaProblemError from capa.util import convert_files_to_filenames from .progress import Progress -from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.exceptions import NotFoundError from xblock.fields import Scope, String, Boolean, Dict, Integer, Float from .fields import Timedelta, Date from django.utils.timezone import UTC @@ -270,11 +268,13 @@ def __init__(self, *args, **kwargs): ) ) # create a dummy problem with error message instead of failing - problem_text = (u'' - u'Problem {url} has an error:{msg}'.format( - url=self.location.to_deprecated_string(), - msg=msg) - ) + problem_text = ( + u'' + u'Problem {url} has an error:{msg}'.format( + url=self.location.to_deprecated_string(), + msg=msg, + ) + ) self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text) else: # add extra info and raise diff --git a/common/lib/xmodule/xmodule/capa_base_constants.py b/common/lib/xmodule/xmodule/capa_base_constants.py index 20eab88a07a5..7739be238e8f 100644 --- a/common/lib/xmodule/xmodule/capa_base_constants.py +++ b/common/lib/xmodule/xmodule/capa_base_constants.py @@ -4,7 +4,7 @@ """ -class SHOWANSWER: +class SHOWANSWER(object): """ Constants for when to show answer """ @@ -18,7 +18,7 @@ class SHOWANSWER: NEVER = "never" -class RANDOMIZATION: +class RANDOMIZATION(object): """ Constants for problem randomization """ diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 03b1f217d563..f98245344c4f 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -186,7 +186,7 @@ def from_json(self, value): version_error_string = "Could not find version {0}, using version {1} instead" log.error(version_error_string.format(value, DEFAULT_VERSION)) value = DEFAULT_VERSION - except: + except Exception: value = DEFAULT_VERSION return value diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index c887670a7a0e..4ad2fca4cdb6 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -239,7 +239,7 @@ def definition_from_xml(cls, xml_object, system): try: descriptor = system.process_xml(etree.tostring(child)) children.append(descriptor.scope_ids.usage_id) - except: + except Exception: msg = "Unable to load child when parsing Conditional." log.exception(msg) system.error_tracker(msg) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index cb91fe4b6b95..4cefc6aac8a7 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -54,7 +54,7 @@ def to_json(self, value): """ try: result = super(StringOrDate, self).to_json(value) - except: + except Exception: return value if result is None: return value @@ -115,7 +115,10 @@ def table_of_contents(self): pass # Get the table of contents from S3 - log.info("Retrieving textbook table of contents from %s" % toc_url) + log.info( + "Retrieving textbook table of contents from %s", + toc_url, + ) try: r = requests.get(toc_url) except Exception as err: @@ -147,7 +150,7 @@ def from_json(self, values): for title, book_url in values: try: textbooks.append(Textbook(title, book_url)) - except: + except Exception: # If we can't get to S3 (e.g. on a train with no internet), don't break # the rest of the courseware. log.exception("Couldn't load textbook ({0}, {1})".format(title, book_url)) @@ -1299,7 +1302,7 @@ def forum_posts_allowed(self): for start, end in blackout_periods: if start <= now <= end: return False - except: + except Exception: log.exception("Error parsing discussion_blackouts for course {0}".format(self.id)) return True diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 1d0d0c56a54b..b3cae8c81dd7 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -328,8 +328,11 @@ def tally_vote(self, data): try: hint_and_votes.append(temp_dict[answer][str(vote_pk)]) except KeyError: - log.exception('In hinter tally_vote, couldn\'t find: {ans}, {vote_pk}'.format( - ans=answer, vote_pk=str(vote_pk))) + log.exception( + "In hinter tally_vote, couldn't find: %s, %s", + answer, + str(vote_pk), + ) hint_and_votes.sort(key=lambda pair: pair[1], reverse=True) # Reset self.previous_answers and user_submissions. @@ -351,8 +354,10 @@ def submit_hint(self, data): hint = escape(data['hint']) answer = data['answer'] if not self.validate_answer(answer): - log.exception('Failure in hinter submit_hint: Unable to parse answer: {ans}'.format( - ans=answer)) + log.exception( + "Failure in hinter submit_hint: Unable to parse answer: %s", + answer, + ) return {'error': 'Could not submit answer'} # Only allow a student to vote or submit a hint once. if self.user_voted: diff --git a/common/lib/xmodule/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py index a6d3686ca2fd..d1f05171da47 100644 --- a/common/lib/xmodule/xmodule/exceptions.py +++ b/common/lib/xmodule/xmodule/exceptions.py @@ -53,4 +53,4 @@ def __init__(self, msg, service): In addition to a msg, provide the name of the service. """ self.service = service - return super(HeartbeatFailure, self).__init__(msg) + super(HeartbeatFailure, self).__init__(msg) diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index b5e0e1ba9f4e..6dad70ddc771 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -201,8 +201,8 @@ class SingleSectionGrader(CourseGrader): If the name is not appropriate for the short short_label or category, they each may be specified individually. """ - def __init__(self, type, name, short_label=None, category=None): - self.type = type + def __init__(self, format_type, name, short_label=None, category=None): + self.type = format_type self.name = name self.short_label = short_label or name self.category = category or name @@ -238,10 +238,11 @@ def grade(self, grade_sheet, generate_random_scores=False): breakdown = [{'percent': percent, 'label': self.short_label, 'detail': detail, 'category': self.category, 'prominent': True}] - return {'percent': percent, - 'section_breakdown': breakdown, - #No grade_breakdown here - } + return { + 'percent': percent, + 'section_breakdown': breakdown, + # No grade_breakdown here + } class AssignmentFormatGrader(CourseGrader): @@ -278,9 +279,19 @@ class AssignmentFormatGrader(CourseGrader): min_count = 2 would produce the labels "Assignment 3", "Assignment 4" """ - def __init__(self, type, min_count, drop_count, category=None, section_type=None, short_label=None, - show_only_average=False, hide_average=False, starting_index=1): - self.type = type + def __init__( + self, + format_type, + min_count, + drop_count, + category=None, + section_type=None, + short_label=None, + show_only_average=False, + hide_average=False, + starting_index=1, + ): + self.type = format_type self.min_count = min_count self.drop_count = drop_count self.category = category or self.type @@ -352,8 +363,12 @@ def total_with_drops(breakdown, drop_count): total_percent, dropped_indices = total_with_drops(breakdown, self.drop_count) for dropped_index in dropped_indices: - breakdown[dropped_index]['mark'] = {'detail': u"The lowest {drop_count} {section_type} scores are dropped." - .format(drop_count=self.drop_count, section_type=self.section_type)} + breakdown[dropped_index]['mark'] = { + 'detail': u"The lowest {drop_count} {section_type} scores are dropped.".format( + drop_count=self.drop_count, + section_type=self.section_type, + ), + } if len(breakdown) == 1: # if there is only one entry in a section, suppress the existing individual entry and the average, @@ -380,7 +395,8 @@ def total_with_drops(breakdown, drop_count): breakdown.append({'percent': total_percent, 'label': total_label, 'detail': total_detail, 'category': self.category, 'prominent': True}) - return {'percent': total_percent, - 'section_breakdown': breakdown, - #No grade_breakdown here - } + return { + 'percent': total_percent, + 'section_breakdown': breakdown, + # No grade_breakdown here + } diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index 340ce84d2454..aab5c946641a 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -69,7 +69,6 @@ from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.x_module import XModule, module_attr -from xmodule.course_module import CourseDescriptor from xmodule.lti_2_util import LTI20ModuleMixin, LTIError from pkg_resources import resource_string from xblock.core import String, Scope, List, XBlock diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 222f9cdceb92..b5102c8b7282 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -198,9 +198,11 @@ def _get_bulk_ops_record(self, course_key, ignore_case=False): if ignore_case: for key, record in self._active_bulk_ops.records.iteritems(): if ( - key.org.lower() == course_key.org.lower() and - key.course.lower() == course_key.course.lower() and - key.run.lower() == course_key.run.lower() + key.org.lower() == course_key.org.lower() + and + key.course.lower() == course_key.course.lower() + and + key.run.lower() == course_key.run.lower() ): return record return self._active_bulk_ops.records[course_key.for_branch(None)] @@ -1043,15 +1045,23 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead): # pylint: disable=invalid-name def __init__( - self, - contentstore=None, - doc_store_config=None, # ignore if passed up - metadata_inheritance_cache_subsystem=None, request_cache=None, - xblock_mixins=(), xblock_select=None, - # temporary parms to enable backward compatibility. remove once all envs migrated - db=None, collection=None, host=None, port=None, tz_aware=True, user=None, password=None, - # allow lower level init args to pass harmlessly - ** kwargs + self, + contentstore=None, + doc_store_config=None, # ignore if passed up + metadata_inheritance_cache_subsystem=None, + request_cache=None, + xblock_mixins=(), + xblock_select=None, + # temporary parms to enable backward compatibility. remove once all envs migrated + db=None, + collection=None, + host=None, + port=None, + tz_aware=True, + user=None, + password=None, + # allow lower level init args to pass harmlessly + ** kwargs ): ''' Set up the error-tracking logic. diff --git a/common/lib/xmodule/xmodule/modulestore/exceptions.py b/common/lib/xmodule/xmodule/modulestore/exceptions.py index d98c744009a5..12be4c398b37 100644 --- a/common/lib/xmodule/xmodule/modulestore/exceptions.py +++ b/common/lib/xmodule/xmodule/modulestore/exceptions.py @@ -50,8 +50,11 @@ def __str__(self, *args, **kwargs): """ Print info about what's duplicated """ - return '{0.store}[{0.collection}] already has {0.element_id}'.format( - self, Exception.__str__(self, *args, **kwargs) + return "{store}[{collection}] already has {element_id} ({exception})".format( + store=self.store, + collection=self.collection, + element_id=self.element_id, + exception=Exception.__str__(self, *args, **kwargs), ) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index a0d80013591f..d4d5cff76677 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -513,7 +513,15 @@ def __init__(self, contentstore, doc_store_config, fs_root, render_template, super(MongoModuleStore, self).__init__(contentstore=contentstore, **kwargs) def do_connection( - db, collection, host, port=27017, tz_aware=True, user=None, password=None, asset_collection=None, **kwargs + db, + collection, + host, + port=27017, + tz_aware=True, + user=None, + password=None, + asset_collection=None, + **kwargs ): """ Create & open the connection, authenticate, and provide pointers to the collection @@ -1127,8 +1135,15 @@ def create_course(self, org, course, run, user_id, fields=None, **kwargs): return xblock def create_xblock( - self, runtime, course_key, block_type, block_id=None, fields=None, - metadata=None, definition_data=None, **kwargs + self, + runtime, + course_key, + block_type, + block_id=None, + fields=None, + metadata=None, + definition_data=None, + **kwargs ): """ Create the new xblock but don't save it. Returns the new module. diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 97191258ad58..f0d6f1633aa5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -92,7 +92,9 @@ def _copy_published_modules_to_course(self, new_course, old_course_loc, source_c # iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g., # course about pages, conditionals) for module in self.source_modulestore.get_items( - source_course_key, revision=ModuleStoreEnum.RevisionOption.published_only, **kwargs + source_course_key, + revision=ModuleStoreEnum.RevisionOption.published_only, + **kwargs ): # don't copy the course again. if module.location != old_course_loc: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py index 06ae54250613..3816757be28c 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/__init__.py @@ -10,9 +10,9 @@ class BlockKey(namedtuple('BlockKey', 'type id')): __slots__ = () - @contract(type="string[>0]") - def __new__(cls, type, id): - return super(BlockKey, cls).__new__(cls, type, id) + @contract(block_type="string[>0]") + def __new__(cls, block_type, block_id): + return super(BlockKey, cls).__new__(cls, block_type, block_id) @classmethod @contract(usage_key=BlockUsageLocator) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index 708fd6be7d92..7943268c0b0c 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -75,8 +75,17 @@ class MongoConnection(object): Segregation of pymongo functions from the data modeling mechanisms for split modulestore. """ def __init__( - self, db, collection, host, port=27017, tz_aware=True, user=None, password=None, - asset_collection=None, retry_wait_time=0.1, **kwargs + self, + db, + collection, + host, + port=27017, + tz_aware=True, + user=None, + password=None, + asset_collection=None, + retry_wait_time=0.1, + **kwargs ): """ Create & open the connection, authenticate, and provide pointers to the collections diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 324f3176981f..705929dcf0f6 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -491,12 +491,16 @@ def _replace_or_append_index(altered_index): continue if search_targets: - if any( - 'search_targets' not in record.index or - field not in record.index['search_targets'] or - record.index['search_targets'][field] != value - for field, value in search_targets.iteritems() - ): + if any([ + ( + 'search_targets' not in record.index + or + field not in record.index['search_targets'] + or + record.index['search_targets'][field] != value + ) + for field, value in search_targets.iteritems() + ]): continue if not hasattr(indexes, 'append'): # Just in time conversion to list from cursor @@ -1380,9 +1384,15 @@ def _generate_block_key(self, course_blocks, category): @contract(returns='XBlock') def create_item( - self, user_id, course_key, block_type, block_id=None, - definition_locator=None, fields=None, - force=False, **kwargs + self, + user_id, + course_key, + block_type, + block_id=None, + definition_locator=None, + fields=None, + force=False, + **kwargs ): """ Add a descriptor to persistence as an element @@ -1577,9 +1587,18 @@ def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, * DEFAULT_ROOT_BLOCK_ID = 'course' def create_course( - self, org, course, run, user_id, master_branch=None, fields=None, - versions_dict=None, search_targets=None, root_category='course', - root_block_id=None, **kwargs + self, + org, + course, + run, + user_id, + master_branch=None, + fields=None, + versions_dict=None, + search_targets=None, + root_category='course', + root_block_id=None, + **kwargs ): """ Create a new entry in the active courses index which points to an existing or new structure. Returns @@ -1633,9 +1652,16 @@ def create_course( ) def _create_courselike( - self, locator, user_id, master_branch, fields=None, - versions_dict=None, search_targets=None, root_category='course', - root_block_id=None, **kwargs + self, + locator, + user_id, + master_branch, + fields=None, + versions_dict=None, + search_targets=None, + root_category='course', + root_block_id=None, + **kwargs ): """ Internal code for creating a course or library @@ -1756,8 +1782,15 @@ def update_item(self, descriptor, user_id, allow_not_found=False, force=False, * ) or descriptor def _update_item_from_fields( - self, user_id, course_key, block_key, partitioned_fields, - definition_locator, allow_not_found, force, **kwargs + self, + user_id, + course_key, + block_key, + partitioned_fields, + definition_locator, + allow_not_found, + force, + **kwargs ): """ Broke out guts of update_item for short-circuited internal use only @@ -2365,7 +2398,12 @@ def delete_course(self, course_key, user_id): @contract(block_map="dict(BlockKey: dict)", block_key=BlockKey) def inherit_settings( - self, block_map, block_key, inherited_settings_map, inheriting_settings=None, inherited_from=None + self, + block_map, + block_key, + inherited_settings_map, + inheriting_settings=None, + inherited_from=None, ): """ Updates block_data with any inheritable setting set by an ancestor and recurses to children. diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 9c8e38073fae..2114616c3d28 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -131,9 +131,16 @@ def update_item(self, descriptor, user_id, allow_not_found=False, force=False, * return item def create_item( - self, user_id, course_key, block_type, block_id=None, - definition_locator=None, fields=None, - force=False, skip_auto_publish=False, **kwargs + self, + user_id, + course_key, + block_type, + block_id=None, + definition_locator=None, + fields=None, + force=False, + skip_auto_publish=False, + **kwargs ): """ See :py:meth `ModuleStoreDraftAndPublished.create_item` diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 4e7b1063c368..ce0e8f10ec54 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -369,18 +369,27 @@ def check_mongo_calls(num_finds=0, num_sends=None): the given int value. """ with check_sum_of_calls( - pymongo.message, - ['query', 'get_more'], - num_finds, - num_finds + pymongo.message, + [ + 'query', + 'get_more', + ], + num_finds, + num_finds, ): if num_sends is not None: with check_sum_of_calls( - pymongo.message, - # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write - ['insert', 'update', 'delete', '_do_batched_write_command', '_do_batched_insert', ], - num_sends, - num_sends + pymongo.message, + # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write + [ + 'insert', + 'update', + 'delete', + '_do_batched_write_command', + '_do_batched_insert', + ], + num_sends, + num_sends, ): yield else: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index a392e2d05d59..f87963bec4dc 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -57,7 +57,7 @@ class TestMixedModuleStore(CourseComparisonTest): ASSET_COLLECTION = 'assetstore' FS_ROOT = DATA_DIR DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' - RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': '' + RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' MONGO_COURSEID = 'MITx/999/2013_Spring' XML_COURSEID1 = 'edX/toy/2012_Fall' @@ -284,19 +284,31 @@ def test_get_modulestore_type(self, default_ms): Make sure we get back the store type we expect for given mappings """ self.initdb(default_ms) - self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.XML_COURSEID1)), ModuleStoreEnum.Type.xml + self.assertEqual( + self.store.get_modulestore_type( + self._course_key_from_string(self.XML_COURSEID1), + ), + ModuleStoreEnum.Type.xml, ) - self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.XML_COURSEID2)), ModuleStoreEnum.Type.xml + self.assertEqual( + self.store.get_modulestore_type( + self._course_key_from_string(self.XML_COURSEID2), + ), + ModuleStoreEnum.Type.xml, ) mongo_ms_type = ModuleStoreEnum.Type.mongo if default_ms == 'draft' else ModuleStoreEnum.Type.split - self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.MONGO_COURSEID)), mongo_ms_type + self.assertEqual( + self.store.get_modulestore_type( + self._course_key_from_string(self.MONGO_COURSEID), + ), + mongo_ms_type, ) # try an unknown mapping, it should be the 'default' store - self.assertEqual(self.store.get_modulestore_type( - SlashSeparatedCourseKey('foo', 'bar', '2012_Fall')), mongo_ms_type + self.assertEqual( + self.store.get_modulestore_type( + SlashSeparatedCourseKey('foo', 'bar', '2012_Fall'), + ), + mongo_ms_type, ) @ddt.data('draft', 'split') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index c20be348ccd5..ab8e4a209135 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -21,7 +21,6 @@ from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xblock.runtime import KeyValueStore from xblock.exceptions import InvalidScopeError -from xblock.plugin import Plugin from xmodule.tests import DATA_DIR from opaque_keys.edx.locations import Location @@ -54,7 +53,7 @@ ASSET_COLLECTION = 'assetstore' FS_ROOT = DATA_DIR # TODO (vshnayder): will need a real fs_root for testing load_item DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' -RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': '' +RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' class ReferenceTestXBlock(XBlock, XModuleMixin): @@ -194,15 +193,14 @@ def test_get_courses(self): courses = self.draft_store.get_courses() assert_equals(len(courses), 6) course_ids = [course.id for course in courses] - for course_key in [ - - SlashSeparatedCourseKey(*fields) - for fields in [ - ['edX', 'simple', '2012_Fall'], ['edX', 'simple_with_draft', '2012_Fall'], - ['edX', 'test_import_course', '2012_Fall'], ['edX', 'test_unicode', '2012_Fall'], - ['edX', 'toy', '2012_Fall'] - ] + for fields in [ + ['edX', 'simple', '2012_Fall'], + ['edX', 'simple_with_draft', '2012_Fall'], + ['edX', 'test_import_course', '2012_Fall'], + ['edX', 'test_unicode', '2012_Fall'], + ['edX', 'toy', '2012_Fall'], ]: + course_key = SlashSeparatedCourseKey(*fields) assert_in(course_key, course_ids) course = self.draft_store.get_course(course_key) assert_not_none(course) @@ -217,14 +215,12 @@ def test_no_such_course(self): """ Test get_course and has_course with ids which don't exist """ - for course_key in [ - - SlashSeparatedCourseKey(*fields) - for fields in [ - ['edX', 'simple', 'no_such_course'], ['edX', 'no_such_course', '2012_Fall'], + for fields in [ + ['edX', 'simple', 'no_such_course'], + ['edX', 'no_such_course', '2012_Fall'], ['NO_SUCH_COURSE', 'Test_iMport_courSe', '2012_Fall'], - ] ]: + course_key = SlashSeparatedCourseKey(*fields) course = self.draft_store.get_course(course_key) assert_is_none(course) assert_false(self.draft_store.has_course(course_key)) 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 8c0750ddf48c..28dbc98b6f18 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -884,10 +884,21 @@ def test_matching(self): self.assertTrue(modulestore()._value_matches(['distract', 'help', 'notme'], 'help')) self.assertFalse(modulestore()._value_matches(['distract', 'Help', 'notme'], 'help')) self.assertFalse(modulestore()._block_matches({'field': ['distract', 'Help', 'notme']}, {'field': 'help'})) - self.assertTrue(modulestore()._block_matches( - {'field': ['distract', 'help', 'notme'], - 'irrelevant': 2}, - {'field': 'help'})) + self.assertTrue( + modulestore()._block_matches( + { + 'field': [ + 'distract', + 'help', + 'notme', + ], + 'irrelevant': 2, + }, + { + 'field': 'help', + }, + ), + ) self.assertTrue(modulestore()._value_matches('I need some help', re.compile(r'help'))) self.assertTrue(modulestore()._value_matches(['I need some help', 'today'], re.compile(r'help'))) self.assertFalse(modulestore()._value_matches('I need some help', re.compile(r'Help'))) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py index cfb9e5aa8377..11396ff8c8a2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py @@ -465,8 +465,12 @@ def active_structure(_id): self.assertNotIn(structure, results) for structure in db_structures: if ( - structure['previous_version'] in search_ids and # We're searching for this document - not any(active.endswith(structure['_id']) for active in active_ids) # This document doesn't match any active _ids + structure['previous_version'] in search_ids # We're searching for this document + and + not any([ + active.endswith(structure['_id']) + for active in active_ids + ]) # This document doesn't match any active _ids ): self.assertIn(structure, results) else: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index a6b227b03b64..3adde6d8d246 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -28,7 +28,7 @@ class ModuleStoreNoSettings(unittest.TestCase): COLLECTION = 'modulestore' FS_ROOT = DATA_DIR DEFAULT_CLASS = 'xmodule.modulestore.tests.test_xml_importer.StubXBlock' - RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': '' + RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' modulestore_options = { 'default_class': DEFAULT_CLASS, diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 05d0eef0aa35..738f7e4826cc 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -432,11 +432,13 @@ def load_course(self, course_dir, course_ids, tracker): course = course_data.get('course') if course is None: - msg = ("No 'course' attribute set for course in {dir}." - " Using default '{default}'".format(dir=course_dir, - default=course_dir - ) - ) + msg = ( + "No 'course' attribute set for course in {dir}." + " Using default '{default}'".format( + dir=course_dir, + default=course_dir, + ) + ) log.warning(msg) tracker(msg) course = course_dir @@ -705,7 +707,7 @@ def make_course_key(self, org, course, run): """ return CourseLocator(org, course, run, deprecated=True) - def get_courses(self, depth=0, **kwargs): + def get_courses(self, **kwargs): """ Returns a list of course descriptors. If there were errors on loading, some of these may be ErrorDescriptors instead. diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 1054520a7eb9..6d569a0569d0 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -940,8 +940,9 @@ def perform_xlint( print("\n") print("------------------------------------------") print("VALIDATION SUMMARY: {err} Errors {warn} Warnings".format( - err=err_cnt, warn=warn_cnt) - ) + err=err_cnt, + warn=warn_cnt, + )) if err_cnt > 0: print( diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 2f3d2f7159cc..3a7794a0c9a8 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -67,7 +67,7 @@ SKIP_BASIC_CHECKS = False -class CombinedOpenEndedV1Module(): +class CombinedOpenEndedV1Module(object): """ This is a module that encapsulates all open ended grading (self assessment, peer assessment, etc). It transitions between problems, and support arbitrary ordering. @@ -1185,7 +1185,7 @@ def service_declaration(cls, service_name): return declaration -class CombinedOpenEndedV1Descriptor(): +class CombinedOpenEndedV1Descriptor(object): """ Module for adding combined open ended questions """ diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py index 16ea4acba896..02071fae3721 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_rubric.py @@ -332,8 +332,11 @@ def reformat_scores_for_rendering(scores, score_types, feedback_types): score_tuples = [] for i in xrange(0, len(score_lists)): for j in xrange(0, len(score_lists[i])): - tuple = [1, j, score_lists[i][j], [], []] - score_tuples, tup_ind = CombinedOpenEndedRubric.check_for_tuple_matches(score_tuples, tuple) + score_tuple = [1, j, score_lists[i][j], [], []] + score_tuples, tup_ind = CombinedOpenEndedRubric.check_for_tuple_matches( + score_tuples, + score_tuple, + ) score_tuples[tup_ind][0] += 1 score_tuples[tup_ind][3].append(score_type_list[i]) score_tuples[tup_ind][4].append(feedback_type_list[i]) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py index e1e7d5214505..f15abc24b06a 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py @@ -1,5 +1,4 @@ # This class gives a common interface for logging into the grading controller -import json import logging import requests import dogstats_wrapper as dog_stats_api 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 b4801ad16b17..83867aeea9fe 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 @@ -170,7 +170,7 @@ def message_post(self, data, system): grader_id = int(survey_responses['grader_id']) feedback = str(survey_responses['feedback'].encode('ascii', 'ignore')) score = int(survey_responses['score']) - except: + except Exception: # This is a dev_facing_error error_message = ( "Could not parse submission id, grader id, " @@ -846,7 +846,7 @@ def score_for_attempt(self, index): return score -class OpenEndedDescriptor(): +class OpenEndedDescriptor(object): """ Module for adding open ended response questions to courses """ diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index f0ac9052b599..1d0927bb3e7c 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -113,7 +113,7 @@ def __init__(self, system, location, definition, descriptor, static_data, if instance_state is not None: try: instance_state = json.loads(instance_state) - except: + except Exception: log.error( "Could not load instance state for open ended. Setting it to nothing.: {0}".format(instance_state)) instance_state = {} @@ -158,7 +158,7 @@ def __init__(self, system, location, definition, descriptor, static_data, self.location_string = location try: self.location_string = self.location_string.to_deprecated_string() - except: + except Exception: pass self.setup_response(system, location, definition, descriptor) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py index da76e1867d8c..1ad247be74d1 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py @@ -117,13 +117,12 @@ def get_notifications(self, course_id, grader_id): return result -""" -This is a mock peer grading service that can be used for unit tests -without making actual service calls to the grading controller -""" - - class MockPeerGradingService(object): + """ + This is a mock peer grading service that can be used for unit tests + without making actual service calls to the grading controller + """ + def get_next_submission(self, problem_location, grader_id): return { 'success': True, diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index e9881db34434..84ca9f32ea08 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py @@ -281,12 +281,12 @@ def latest_post_assessment(self, system): latest_post_assessment = super(SelfAssessmentModule, self).latest_post_assessment(system) try: rubric_scores = json.loads(latest_post_assessment) - except: + except Exception: rubric_scores = [] return [rubric_scores] -class SelfAssessmentDescriptor(): +class SelfAssessmentDescriptor(object): """ Module for adding self assessment questions to courses """ diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index f275977a7b89..afc3e34fcbcc 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -261,7 +261,7 @@ def get_score(self): try: count_graded = self.student_data_for_location['count_graded'] count_required = self.student_data_for_location['count_required'] - except: + except Exception: success, response = self.query_data_for_location(self.link_to_location) if not success: log.exception( @@ -616,7 +616,7 @@ def peer_grading_problem(self, data=None): elif data.get('location') is not None: problem_location = self.course_id.make_usage_key_from_deprecated_string(data.get('location')) - module = self._find_corresponding_module_for_location(problem_location) # pylint: disable-unused-variable + self._find_corresponding_module_for_location(problem_location) ajax_url = self.ajax_url html = self.system.render_template('peer_grading/peer_grading_problem.html', { diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index 0f6235dc1e70..379ac3d1e014 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -69,15 +69,23 @@ def handle_ajax(self, dispatch, data): self.voted = True self.poll_answer = dispatch - return json.dumps({'poll_answers': self.poll_answers, - 'total': sum(self.poll_answers.values()), - 'callback': {'objectName': 'Conditional'} - }) + return json.dumps( + { + 'poll_answers': self.poll_answers, + 'total': sum(self.poll_answers.values()), + 'callback': { + 'objectName': 'Conditional', + }, + }, + ) elif dispatch == 'get_state': - return json.dumps({'poll_answer': self.poll_answer, - 'poll_answers': self.poll_answers, - 'total': sum(self.poll_answers.values()) - }) + return json.dumps( + { + 'poll_answer': self.poll_answer, + 'poll_answers': self.poll_answers, + 'total': sum(self.poll_answers.values()), + }, + ) elif dispatch == 'reset_poll' and self.voted and \ self.descriptor.xml_attributes.get('reset', 'True').lower() != 'false': self.voted = False diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 4c4d9b26afdd..4e9aea22a2b2 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -138,13 +138,14 @@ def student_view(self, context): childinfo['title'] = child.display_name_with_default contents.append(childinfo) - params = {'items': contents, - 'element_id': self.location.html_id(), - 'item_id': self.location.to_deprecated_string(), - 'position': self.position, - 'tag': self.location.category, - 'ajax_url': self.system.ajax_url, - } + params = { + 'items': contents, + 'element_id': self.location.html_id(), + 'item_id': self.location.to_deprecated_string(), + 'position': self.position, + 'tag': self.location.category, + 'ajax_url': self.system.ajax_url, + } fragment.add_content(self.system.render_template('seq_module.html', params)) diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index fb0926950a1f..9020f058f0a5 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -1,12 +1,6 @@ """ Implement CourseTab """ -# pylint: disable=incomplete-protocol -# Note: pylint complains that we do not implement __delitem__ and __len__, although we implement __setitem__ -# and __getitem__. However, the former two do not apply to the CourseTab class so we do not implement them. -# The reason we implement the latter two is to enable callers to continue to use the CourseTab object with -# dict-type accessors. - from abc import ABCMeta, abstractmethod from xblock.fields import List @@ -15,7 +9,7 @@ _ = lambda text: text -class CourseTab(object): # pylint: disable=incomplete-protocol +class CourseTab(object): """ The Course Tab class is a data abstraction for all tabs (i.e., course navigation links) within a course. It is an abstract class - to be inherited by various tab types. diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index d1309fded457..1869b8d431aa 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -79,14 +79,15 @@ def answer_key(cls, response_num=2, input_num=1): ) @classmethod - def create(cls, - attempts=None, - problem_state=None, - correct=False, - xml=None, - override_get_score=True, - **kwargs - ): + def create( + cls, + attempts=None, + problem_state=None, + correct=False, + xml=None, + override_get_score=True, + **kwargs + ): """ All parameters are optional, and are added to the created problem if specified. @@ -1027,10 +1028,14 @@ def test_check_button_name_customization(self): ) self.assertEqual(module.check_button_name(), "Submit") - module = CapaFactory.create(attempts=9, - max_attempts=10, - text_customization={"custom_check": "Submit", "custom_final_check": "Final Submit"} - ) + module = CapaFactory.create( + attempts=9, + max_attempts=10, + text_customization={ + 'custom_check': 'Submit', + 'custom_final_check': 'Final Submit', + }, + ) self.assertEqual(module.check_button_name(), "Final Submit") def test_check_button_checking_name_customization(self): diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index f3eea8bbc8a3..54adb9cf6ffb 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -50,13 +50,13 @@ """ -class Content: +class Content(object): def __init__(self, location, content_type): self.location = location self.content_type = content_type -class FakeGridFsItem: +class FakeGridFsItem(object): """ This class provides the basic methods to get data from a GridFS item """ diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index a469831fb9ff..db0d831fb49c 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -60,7 +60,8 @@ def to_attrb(n, v): advertised_start = to_attrb('advertised_start', advertised_start) end = to_attrb('end', end) - start_xml = ''' + start_xml = ( + ''' Two houses, ... - '''.format( + ''' + ).format( org=ORG, course=COURSE, start=start, diff --git a/common/lib/xmodule/xmodule/tests/test_delay_between_attempts.py b/common/lib/xmodule/xmodule/tests/test_delay_between_attempts.py index 9a239f0860bb..127491eea62d 100644 --- a/common/lib/xmodule/xmodule/tests/test_delay_between_attempts.py +++ b/common/lib/xmodule/xmodule/tests/test_delay_between_attempts.py @@ -74,12 +74,12 @@ def answer_key(cls, input_num=2): @classmethod def create( - cls, - max_attempts=None, - attempts=None, - correct=False, - last_submission_time=None, - submission_wait_seconds=None + cls, + max_attempts=None, + attempts=None, + correct=False, + last_submission_time=None, + submission_wait_seconds=None, ): """ Optional parameters here are cut down to what we actually use vs. the regular CapaFactory. diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index 28bc81db5591..1b12017fbde6 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -119,8 +119,8 @@ class TestErrorModuleConstruction(unittest.TestCase): """ Test that error module construction happens correctly """ - def setUp(self): + # pylint: disable=abstract-class-instantiated super(TestErrorModuleConstruction, self).setUp() field_data = Mock(spec=FieldData) self.descriptor = BrokenDescriptor( diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index fd66869dbab9..a50f6f3270fe 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -4,7 +4,6 @@ from django.utils.timezone import UTC from xmodule.fields import Date, Timedelta, RelativeTime from xmodule.timeinfo import TimeInfo -import time class DateTest(unittest.TestCase): diff --git a/common/lib/xmodule/xmodule/tests/test_tabs.py b/common/lib/xmodule/xmodule/tests/test_tabs.py index 3c9b54bb43b6..52524e4f58e0 100644 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ b/common/lib/xmodule/xmodule/tests/test_tabs.py @@ -92,12 +92,12 @@ def check_tab_json_methods(self, tab): self.assertEquals(serialized_tab, deserialized_tab) def check_can_display_results( - self, - tab, - expected_value=True, - for_authenticated_users_only=False, - for_staff_only=False, - for_enrolled_users_only=False + self, + tab, + expected_value=True, + for_authenticated_users_only=False, + for_staff_only=False, + for_enrolled_users_only=False, ): """Checks can display results for various users""" if for_staff_only: @@ -615,15 +615,15 @@ def test_iterate_displayable(self): # enumerate the tabs using the CMS call for i, tab in enumerate(tabs.CourseTabList.iterate_displayable_cms( - self.course, - self.settings, + self.course, + self.settings, )): self.assertEquals(tab.type, self.course.tabs[i].type) # enumerate the tabs and verify textbooks and the instructor tab for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( - self.course, - self.settings, + self.course, + self.settings, )): if getattr(tab, 'is_collection_item', False): # a collection item was found as a result of a collection tab @@ -688,12 +688,13 @@ def reverse_discussion_link(viewname, args): return reverse_discussion_link def check_discussion( - self, tab_list, - expected_discussion_link, - expected_can_display_value, - discussion_link_in_course="", - is_staff=True, - is_enrolled=True, + self, + tab_list, + expected_discussion_link, + expected_can_display_value, + discussion_link_in_course='', + is_staff=True, + is_enrolled=True, ): """Helper function to verify whether the discussion tab exists and can be displayed""" self.course.tabs = tab_list diff --git a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py index 0f797b1d63fd..5f98fccce9cf 100644 --- a/common/lib/xmodule/xmodule/tests/test_util_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_util_open_ended.py @@ -1,7 +1,6 @@ import json -from textwrap import dedent from xmodule.modulestore.xml import XMLModuleStore -from xmodule.tests import DATA_DIR, get_test_system +from xmodule.tests import DATA_DIR from StringIO import StringIO diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index c5916eefd2b1..2f5442f394f0 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -258,7 +258,8 @@ def non_editable_metadata_fields(self): return system.construct_xblock_from_class(TestModuleDescriptor, field_data=field_data, scope_ids=Mock()) def assert_field_values(self, editable_fields, name, field, explicitly_set, value, default_value, - type='Generic', options=[]): + type='Generic', options=None): + options = options or [] test_field = editable_fields[name] self.assertEqual(field.name, test_field['field_name']) diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index d785edec5bec..04c7d2d28bf1 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -4,7 +4,6 @@ StudentViewHandlers are handlers for video module instance. StudioViewHandlers are handlers for video descriptor instance. """ -import os import json import logging from webob import Response @@ -220,9 +219,9 @@ def transcript(self, request, dispatch): # if no translation is required return self.get_static_transcript(request) except ( - TranscriptException, - UnicodeDecodeError, - TranscriptsGenerationException + TranscriptException, + UnicodeDecodeError, + TranscriptsGenerationException, ) as ex: log.info(ex.message) response = Response(status=404) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 892e0a99aedb..f6b8402fd09f 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1178,7 +1178,12 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): # p """ def __init__( - self, load_item, resources_fs, error_tracker, get_policy=None, **kwargs + self, + load_item, + resources_fs, + error_tracker, + get_policy=None, + **kwargs ): """ load_item: Takes a Location and returns an XModuleDescriptor diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index cfe5238b0336..82d1f1fcb924 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -12,7 +12,6 @@ from edxmako.shortcuts import render_to_response from branding.views import index -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from edxmako.tests import mako_middleware_process_request import student.views from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index b79df70f557f..77b9849e4962 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -45,8 +45,11 @@ def index(request): # In this case, we want to have the user stay on a course catalog # page to make it easier to browse for courses (and register) if microsite.get_value( - 'ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER', - settings.FEATURES.get('ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER', True) + 'ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER', + settings.FEATURES.get( + 'ALWAYS_REDIRECT_HOMEPAGE_TO_DASHBOARD_FOR_AUTHENTICATED_USER', + True + ) ): return redirect(reverse('dashboard')) diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index 49fb840560e8..e9ef4c94136b 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email/forms.py @@ -17,12 +17,12 @@ log = logging.getLogger(__name__) -class CourseEmailTemplateForm(forms.ModelForm): # pylint: disable=incomplete-protocol +class CourseEmailTemplateForm(forms.ModelForm): """Form providing validation of CourseEmail templates.""" name = forms.CharField(required=False) - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring model = CourseEmailTemplate fields = ('html_template', 'plain_template', 'name') @@ -73,10 +73,10 @@ def clean_name(self): return name -class CourseAuthorizationAdminForm(forms.ModelForm): # pylint: disable=incomplete-protocol +class CourseAuthorizationAdminForm(forms.ModelForm): """Input form for email enabling, allowing us to verify data.""" - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring model = CourseAuthorization def clean_course_id(self): diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index df59c8fc1d60..e9ab0b52c482 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -44,7 +44,7 @@ class Email(models.Model): created = models.DateTimeField(auto_now_add=True) modified = models.DateTimeField(auto_now=True) - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring abstract = True @@ -142,7 +142,7 @@ class Optout(models.Model): user = models.ForeignKey(User, db_index=True, null=True) course_id = CourseKeyField(max_length=255, db_index=True) - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring unique_together = ('user', 'course_id') diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 22101b46c56e..0e2284a832b2 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -33,7 +33,8 @@ from django.core.urlresolvers import reverse from bulk_email.models import ( - CourseEmail, Optout, CourseEmailTemplate, + CourseEmail, + Optout, SEND_TO_MYSELF, SEND_TO_ALL, TO_OPTIONS, ) from courseware.courses import get_course, course_image_url @@ -91,7 +92,7 @@ ) -def _get_recipient_queryset(user_id, to_option, course_id, course_location): +def _get_recipient_queryset(user_id, to_option, course_id): """ Returns a query set of email recipients corresponding to the requested to_option category. @@ -229,7 +230,7 @@ def _create_send_email_subtask(to_list, initial_subtask_status): ) return new_subtask - recipient_qset = _get_recipient_queryset(user_id, to_option, course_id, course.location) + recipient_qset = _get_recipient_queryset(user_id, to_option, course_id) recipient_fields = ['profile__name', 'email'] log.info(u"Task %s: Preparing to queue subtasks for sending emails for course %s, email %s, to_option %s", diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 3f0fc7ea167a..61e7e24401f0 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -9,9 +9,7 @@ from django.core.management import call_command from django.core.urlresolvers import reverse from django.conf import settings -from django.test.utils import override_settings -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory from student.models import CourseEnrollment from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 5dba89ff1e13..196bce167fa0 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -15,7 +15,6 @@ from bulk_email.models import Optout from courseware.tests.factories import StaffFactory, InstructorFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from instructor_task.subtasks import update_subtask_status from student.roles import CourseStaffRole from student.models import CourseEnrollment diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index ddc728f52c0c..dd01b2433056 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -5,7 +5,6 @@ from itertools import cycle from celery.states import SUCCESS, RETRY -from django.test.utils import override_settings from django.conf import settings from django.core.management import call_command from django.core.urlresolvers import reverse @@ -16,7 +15,6 @@ from bulk_email.models import CourseEmail, SEND_TO_ALL from bulk_email.tasks import perform_delegate_email_batches, send_course_email -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from instructor_task.models import InstructorTask from instructor_task.subtasks import ( initialize_subtask_info, diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index 2328dd271ee0..c0d4ceb68f5a 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -3,13 +3,12 @@ Unit tests for bulk-email-related forms. """ from django.conf import settings -from django.test.utils import override_settings from mock import patch from bulk_email.models import CourseAuthorization, CourseEmailTemplate from bulk_email.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MOCK_MODULESTORE, TEST_DATA_MIXED_TOY_MODULESTORE + TEST_DATA_MIXED_TOY_MODULESTORE, ) from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/certificates/management/commands/cert_whitelist.py b/lms/djangoapps/certificates/management/commands/cert_whitelist.py index 1af687325087..427caced948f 100644 --- a/lms/djangoapps/certificates/management/commands/cert_whitelist.py +++ b/lms/djangoapps/certificates/management/commands/cert_whitelist.py @@ -74,7 +74,7 @@ def handle(self, *args, **options): else: user = User.objects.get(username=user_str) - cert_whitelist, created = \ + cert_whitelist, _created = \ CertificateWhitelist.objects.get_or_create( user=user, course_id=course) if options['add']: diff --git a/lms/djangoapps/certificates/management/commands/gen_cert_report.py b/lms/djangoapps/certificates/management/commands/gen_cert_report.py index 60524387eaf4..78cb7981e928 100644 --- a/lms/djangoapps/certificates/management/commands/gen_cert_report.py +++ b/lms/djangoapps/certificates/management/commands/gen_cert_report.py @@ -6,12 +6,9 @@ from certificates.models import GeneratedCertificate from django.contrib.auth.models import User from optparse import make_option -from django.conf import settings from opaque_keys import InvalidKeyError -from xmodule.course_module import CourseDescriptor from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.modulestore.django import modulestore from django.db.models import Count @@ -93,8 +90,11 @@ def handle(self, *args, **options): ) cert_data[course_id].update( - {status['status']: status['dcount'] - for status in status_tally}) + { + status['status']: status['dcount'] + for status in status_tally + }, + ) mode_tally = GeneratedCertificate.objects.filter( course_id__exact=course_id, @@ -103,21 +103,31 @@ def handle(self, *args, **options): dcount=Count('mode') ) cert_data[course_id].update( - {mode['mode']: mode['dcount'] - for mode in mode_tally} + { + mode['mode']: mode['dcount'] + for mode in mode_tally + }, ) # all states we have seen far all courses - status_headings = sorted(set( - [status for course in cert_data - for status in cert_data[course]]) + status_headings = sorted( + set( + [ + [ + status + for status in cert_data[course] + ] + for course in cert_data + ] + ) ) # print the heading for the report print "{:>26}".format("course ID"), - print ' '.join(["{:>16}".format(heading) - for heading in status_headings] - ) + print ' '.join([ + "{:>16}".format(heading) + for heading in status_headings + ]) # print the report print "{0:>26}".format(course_id.to_deprecated_string()), diff --git a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py index 838e4224a3b0..53d2bfed2932 100644 --- a/lms/djangoapps/certificates/management/commands/ungenerated_certs.py +++ b/lms/djangoapps/certificates/management/commands/ungenerated_certs.py @@ -81,7 +81,7 @@ def handle(self, *args, **options): # Print update after this many students - STATUS_INTERVAL = 500 + status_interval = 500 if options['course']: # try to parse out the course from the serialized form @@ -108,23 +108,23 @@ def handle(self, *args, **options): courseenrollment__course_id=course_key ) - xq = XQueueCertInterface() + xqueue = XQueueCertInterface() if options['insecure']: - xq.use_https = False + xqueue.use_https = False total = enrolled_students.count() count = 0 start = datetime.datetime.now(UTC) for student in enrolled_students: count += 1 - if count % STATUS_INTERVAL == 0: + if count % status_interval == 0: # Print a status update with an approximation of # how much time is left based on how long the last # interval took diff = datetime.datetime.now(UTC) - start - timeleft = diff * (total - count) / STATUS_INTERVAL + timeleft = diff * (total - count) / status_interval hours, remainder = divmod(timeleft.seconds, 3600) - minutes, seconds = divmod(remainder, 60) + minutes, _seconds = divmod(remainder, 60) print "{0}/{1} completed ~{2:02}:{3:02}m remaining".format( count, total, hours, minutes) start = datetime.datetime.now(UTC) @@ -144,7 +144,7 @@ def handle(self, *args, **options): if not options['noop']: # Add the certificate request to the queue - ret = xq.add_cert(student, course_key, course=course) + ret = xqueue.add_cert(student, course_key, course=course) if ret == 'generating': LOGGER.info( diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 0476579da597..1f4000afc61f 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -1,13 +1,3 @@ -from django.contrib.auth.models import User -from django.db import models -from django.db.models.signals import post_save -from django.dispatch import receiver -from django.conf import settings -from datetime import datetime -from model_utils import Choices -from xmodule_django.models import CourseKeyField, NoneToEmptyManager -from util.milestones_helpers import fulfill_course_milestone - """ Certificates are created for a student and an offering of a course. @@ -55,6 +45,16 @@ unless he has allow_certificate set to False. """ +from django.contrib.auth.models import User +from django.db import models +from django.db.models.signals import post_save +from django.dispatch import receiver +from django.conf import settings +from datetime import datetime +from model_utils import Choices +from xmodule_django.models import CourseKeyField, NoneToEmptyManager +from util.milestones_helpers import fulfill_course_milestone + class CertificateStatuses(object): deleted = 'deleted' @@ -105,7 +105,7 @@ class GeneratedCertificate(models.Model): auto_now=True, default=datetime.now) error_reason = models.CharField(max_length=512, blank=True, default='') - class Meta: + class Meta(object): unique_together = (('user', 'course_id'),) @classmethod @@ -165,14 +165,16 @@ def certificate_status_for_student(student, course_id): try: generated_certificate = GeneratedCertificate.objects.get( user=student, course_id=course_id) - d = {'status': generated_certificate.status, - 'mode': generated_certificate.mode} + data = { + 'status': generated_certificate.status, + 'mode': generated_certificate.mode, + } if generated_certificate.grade: - d['grade'] = generated_certificate.grade + data['grade'] = generated_certificate.grade if generated_certificate.status == CertificateStatuses.downloadable: - d['download_url'] = generated_certificate.download_url + data['download_url'] = generated_certificate.download_url - return d + return data except GeneratedCertificate.DoesNotExist: pass return {'status': CertificateStatuses.unavailable, 'mode': GeneratedCertificate.MODES.honor} diff --git a/lms/djangoapps/certificates/tests/factories.py b/lms/djangoapps/certificates/tests/factories.py index d800032f669e..21311c5598c6 100644 --- a/lms/djangoapps/certificates/tests/factories.py +++ b/lms/djangoapps/certificates/tests/factories.py @@ -1,7 +1,5 @@ from factory.django import DjangoModelFactory -from opaque_keys.edx.locations import SlashSeparatedCourseKey - from certificates.models import GeneratedCertificate, CertificateStatuses diff --git a/lms/djangoapps/certificates/tests/tests.py b/lms/djangoapps/certificates/tests/tests.py index eeb68976d1b1..25b63663ef48 100644 --- a/lms/djangoapps/certificates/tests/tests.py +++ b/lms/djangoapps/certificates/tests/tests.py @@ -4,7 +4,6 @@ from mock import patch from django.conf import settings -from django.test import TestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/certificates/views.py b/lms/djangoapps/certificates/views.py index 13d341dc1aab..b52aef2ce280 100644 --- a/lms/djangoapps/certificates/views.py +++ b/lms/djangoapps/certificates/views.py @@ -10,11 +10,10 @@ from capa.xqueue_interface import XQUEUE_METRIC_NAME from certificates.models import certificate_status_for_student, CertificateStatuses, GeneratedCertificate from certificates.queue import XQueueCertInterface -from xmodule.course_module import CourseDescriptor from xmodule.modulestore.django import modulestore from opaque_keys.edx.locations import SlashSeparatedCourseKey -logger = logging.getLogger(__name__) +LOGGER = logging.getLogger(__name__) @csrf_exempt @@ -37,7 +36,7 @@ def request_certificate(request): status = certificate_status_for_student(student, course_key)['status'] if status in [CertificateStatuses.unavailable, CertificateStatuses.notpassing, CertificateStatuses.error]: log_msg = u'Grading and certification requested for user %s in course %s via /request_certificate call' - logger.info(log_msg, username, course_key) + LOGGER.info(log_msg, username, course_key) status = xqci.add_cert(student, course_key, course=course) return HttpResponse(json.dumps({'add_status': status}), mimetype='application/json') return HttpResponse(json.dumps({'add_status': 'ERRORANONYMOUSUSER'}), mimetype='application/json') @@ -69,15 +68,20 @@ def update_certificate(request): key=xqueue_header['lms_key']) except GeneratedCertificate.DoesNotExist: - logger.critical('Unable to lookup certificate\n' + LOGGER.critical('Unable to lookup certificate\n' 'xqueue_body: {0}\n' 'xqueue_header: {1}'.format( xqueue_body, xqueue_header)) - return HttpResponse(json.dumps({ - 'return_code': 1, - 'content': 'unable to lookup key'}), - mimetype='application/json') + return HttpResponse( + json.dumps( + { + 'return_code': 1, + 'content': 'unable to lookup key', + }, + ), + mimetype='application/json', + ) if 'error' in xqueue_body: cert.status = status.error @@ -103,7 +107,7 @@ def update_certificate(request): elif cert.status in [status.deleting]: cert.status = status.deleted else: - logger.critical('Invalid state for cert update: {0}'.format( + LOGGER.critical('Invalid state for cert update: {0}'.format( cert.status)) return HttpResponse( json.dumps({ diff --git a/lms/djangoapps/circuit/views.py b/lms/djangoapps/circuit/views.py index 5af9ce1b4456..a239a025fe20 100644 --- a/lms/djangoapps/circuit/views.py +++ b/lms/djangoapps/circuit/views.py @@ -15,10 +15,11 @@ def circuit_line(circuit): if not circuit.isalnum(): raise Http404() try: - sc = ServerCircuit.objects.get(name=circuit) - schematic = sc.schematic - except: + server_circuit = ServerCircuit.objects.get(name=circuit) + except Exception: schematic = '' + else: + schematic = server_circuit.schematic circuit_line = xml.etree.ElementTree.Element('input') circuit_line.set('type', 'hidden') @@ -31,11 +32,11 @@ def circuit_line(circuit): return xml.etree.ElementTree.tostring(circuit_line) -def edit_circuit(request, circuit): +def edit_circuit(_request, circuit): try: - sc = ServerCircuit.objects.get(name=circuit) - except: - sc = None + server_circuit = ServerCircuit.objects.get(name=circuit) + except Exception: + server_circuit = None if not circuit.isalnum(): raise Http404() @@ -52,13 +53,13 @@ def save_circuit(request, circuit): schematic = request.POST['schematic'] print schematic try: - sc = ServerCircuit.objects.get(name=circuit) - except: - sc = ServerCircuit() - sc.name = circuit - sc.schematic = schematic - print ":", sc.schematic - sc.save() + server_circuit = ServerCircuit.objects.get(name=circuit) + except Exception: + server_circuit = ServerCircuit() + server_circuit.name = circuit + server_circuit.schematic = schematic + print ":", server_circuit.schematic + server_circuit.save() json_str = json.dumps({'results': 'success'}) response = HttpResponse(json_str, mimetype='application/json') response['Cache-Control'] = 'no-cache' diff --git a/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py b/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py index a123236681a9..0e81c149eeff 100644 --- a/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py +++ b/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py @@ -4,24 +4,28 @@ import json -from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.test.client import RequestFactory from mock import patch from capa.tests.response_xml_factory import StringResponseXMLFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from courseware.tests.factories import StudentModuleFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory, AdminFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from class_dashboard.dashboard_data import (get_problem_grade_distribution, get_sequential_open_distrib, - get_problem_set_grade_distrib, get_d3_problem_grade_distrib, - get_d3_sequential_open_distrib, get_d3_section_grade_distrib, - get_section_display_name, get_array_section_has_problem, - get_students_opened_subsection, get_students_problem_grades, - ) +from class_dashboard.dashboard_data import ( + get_problem_grade_distribution, + get_sequential_open_distrib, + get_problem_set_grade_distrib, + get_d3_problem_grade_distrib, + get_d3_sequential_open_distrib, + get_d3_section_grade_distrib, + get_section_display_name, + get_array_section_has_problem, + get_students_opened_subsection, + get_students_problem_grades, +) from class_dashboard.views import has_instructor_access_for_class USER_COUNT = 11 @@ -255,11 +259,14 @@ def test_post_metrics_data_subsections_csv(self): course_id = self.course.id data_type = 'subsection' - data = json.dumps({'sections': sections, - 'tooltips': tooltips, - 'course_id': course_id.to_deprecated_string(), - 'data_type': data_type, - }) + data = json.dumps( + { + 'sections': sections, + 'tooltips': tooltips, + 'course_id': course_id.to_deprecated_string(), + 'data_type': data_type, + }, + ) response = self.client.post(url, {'data': data}) # Check response contains 1 line for header, 1 line for Section and 1 line for Subsection @@ -291,11 +298,14 @@ def test_post_metrics_data_problems_csv(self): course_id = self.course.id data_type = 'problem' - data = json.dumps({'sections': sections, - 'tooltips': tooltips, - 'course_id': course_id.to_deprecated_string(), - 'data_type': data_type, - }) + data = json.dumps( + { + 'sections': sections, + 'tooltips': tooltips, + 'course_id': course_id.to_deprecated_string(), + 'data_type': data_type, + }, + ) response = self.client.post(url, {'data': data}) # Check response contains 1 line for header, 1 line for Sections and 2 lines for problems diff --git a/lms/djangoapps/class_dashboard/tests/test_views.py b/lms/djangoapps/class_dashboard/tests/test_views.py index 765eb4e92e98..4978e0aa6bcf 100644 --- a/lms/djangoapps/class_dashboard/tests/test_views.py +++ b/lms/djangoapps/class_dashboard/tests/test_views.py @@ -1,12 +1,10 @@ """ Tests for class dashboard (Metrics tab in instructor dashboard) """ -from django.test.utils import override_settings from django.test.client import RequestFactory from django.utils import simplejson from mock import patch -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import AdminFactory from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/class_dashboard/urls.py b/lms/djangoapps/class_dashboard/urls.py index fbb3eaf20749..841f6b2cae2b 100644 --- a/lms/djangoapps/class_dashboard/urls.py +++ b/lms/djangoapps/class_dashboard/urls.py @@ -6,27 +6,47 @@ from django.conf import settings COURSE_ID_PATTERN = settings.COURSE_ID_PATTERN -urlpatterns = patterns('', # nopep8 +urlpatterns = patterns( + '', + # Json request data for metrics for entire course - url(r'^{}/all_sequential_open_distrib$'.format(settings.COURSE_ID_PATTERN), - 'class_dashboard.views.all_sequential_open_distrib', name="all_sequential_open_distrib"), + url( + r'^{}/all_sequential_open_distrib$'.format(settings.COURSE_ID_PATTERN), + 'class_dashboard.views.all_sequential_open_distrib', + name='all_sequential_open_distrib', + ), - url(r'^{}/all_problem_grade_distribution$'.format(settings.COURSE_ID_PATTERN), - 'class_dashboard.views.all_problem_grade_distribution', name="all_problem_grade_distribution"), + url( + r'^{}/all_problem_grade_distribution$'.format(settings.COURSE_ID_PATTERN), + 'class_dashboard.views.all_problem_grade_distribution', + name='all_problem_grade_distribution', + ), # Json request data for metrics for particular section - url(r'^{}/problem_grade_distribution/(?P
\d+)$'.format(settings.COURSE_ID_PATTERN), - 'class_dashboard.views.section_problem_grade_distrib', name="section_problem_grade_distrib"), + url( + r'^{}/problem_grade_distribution/(?P
\d+)$'.format(settings.COURSE_ID_PATTERN), + 'class_dashboard.views.section_problem_grade_distrib', + name='section_problem_grade_distrib', + ), # For listing students that opened a sub-section - url(r'^get_students_opened_subsection$', - 'class_dashboard.dashboard_data.get_students_opened_subsection', name="get_students_opened_subsection"), + url( + r'^get_students_opened_subsection$', + 'class_dashboard.dashboard_data.get_students_opened_subsection', + name='get_students_opened_subsection', + ), # For listing of students' grade per problem - url(r'^get_students_problem_grades$', - 'class_dashboard.dashboard_data.get_students_problem_grades', name="get_students_problem_grades"), + url( + r'^get_students_problem_grades$', + 'class_dashboard.dashboard_data.get_students_problem_grades', + name='get_students_problem_grades', + ), # For generating metrics data as a csv - url(r'^post_metrics_data_csv_url', - 'class_dashboard.dashboard_data.post_metrics_data_csv', name="post_metrics_data_csv"), + url( + r'^post_metrics_data_csv_url', + 'class_dashboard.dashboard_data.post_metrics_data_csv', + name='post_metrics_data_csv', + ), ) diff --git a/lms/djangoapps/course_wiki/editors.py b/lms/djangoapps/course_wiki/editors.py index 3708cec1af98..ac6036597e9c 100644 --- a/lms/djangoapps/course_wiki/editors.py +++ b/lms/djangoapps/course_wiki/editors.py @@ -27,10 +27,13 @@ def render(self, name, value, attrs=None): # TODO use the help_text field of edit form instead of rendering a template - return render_to_string('wiki/includes/editor_widget.html', - {'attrs': mark_safe(flatatt(final_attrs)), - 'content': conditional_escape(force_unicode(value)), - }) + return render_to_string( + 'wiki/includes/editor_widget.html', + { + 'attrs': mark_safe(flatatt(final_attrs)), + 'content': conditional_escape(force_unicode(value)), + }, + ) class CodeMirror(BaseEditor): @@ -42,23 +45,25 @@ def get_admin_widget(self, instance=None): def get_widget(self, instance=None): return CodeMirrorWidget() - class AdminMedia: + class AdminMedia(object): css = { 'all': ("wiki/markitup/skins/simple/style.css", "wiki/markitup/sets/admin/style.css",) } - js = ("wiki/markitup/admin.init.js", - "wiki/markitup/jquery.markitup.js", - "wiki/markitup/sets/admin/set.js", - ) + js = ( + 'wiki/markitup/admin.init.js', + 'wiki/markitup/jquery.markitup.js', + 'wiki/markitup/sets/admin/set.js', + ) - class Media: + class Media(object): css = { 'all': ("js/vendor/CodeMirror/codemirror.css",) } - js = ("js/vendor/CodeMirror/codemirror.js", - "js/vendor/CodeMirror/addons/xml.js", - "js/vendor/CodeMirror/addons/edx_markdown.js", - "js/wiki/accessible.js", - "js/wiki/CodeMirror.init.js", - ) + js = ( + 'js/vendor/CodeMirror/codemirror.js', + 'js/vendor/CodeMirror/addons/xml.js', + 'js/vendor/CodeMirror/addons/edx_markdown.js', + 'js/wiki/accessible.js', + 'js/wiki/CodeMirror.init.js', + ) diff --git a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_circuit.py b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_circuit.py index f90cb61123bf..ab5632e21026 100755 --- a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_circuit.py +++ b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_circuit.py @@ -23,7 +23,7 @@ # Markdown 2.1.0 changed from 2.0.3. We try importing the new version first, # but import the 2.0.3 version if it fails from markdown.util import etree -except: +except Exception: from markdown import etree @@ -50,9 +50,9 @@ class CircuitPreprocessor(markdown.preprocessors.Preprocessor): def run(self, lines): def convertLine(line): - m = self.preRegex.match(line) - if m: - return 'processed-schematic:{0}processed-schematic-end'.format(m.group('data')) + match = self.preRegex.match(line) + if match: + return 'processed-schematic:{0}processed-schematic-end'.format(match.group('data')) else: return line @@ -60,8 +60,8 @@ def convertLine(line): class CircuitLink(markdown.inlinepatterns.Pattern): - def handleMatch(self, m): - data = m.group('data') + def handleMatch(self, match): + data = match.group('data') data = escape(data) return etree.fromstring("
") diff --git a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_image.py b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_image.py index 6abfede52f00..8e926c08e3bd 100755 --- a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_image.py +++ b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_image.py @@ -22,7 +22,7 @@ # Markdown 2.1.0 changed from 2.0.3. We try importing the new version first, # but import the 2.0.3 version if it fails from markdown.util import etree -except: +except Exception: from markdown import etree diff --git a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_mathjax.py b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_mathjax.py index b14803744b70..5592fb675336 100644 --- a/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_mathjax.py +++ b/lms/djangoapps/course_wiki/plugins/markdownedx/mdx_mathjax.py @@ -5,7 +5,7 @@ # Markdown 2.1.0 changed from 2.0.3. We try importing the new version first, # but import the 2.0.3 version if it fails from markdown.util import etree, AtomicString -except: +except Exception: from markdown import etree, AtomicString @@ -15,9 +15,9 @@ def __init__(self): markdown.inlinepatterns.Pattern.__init__(self, r'(?[\w\-]+)/update$', 'update_thread', name='update_thread'), - url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), - url(r'threads/(?P[\w\-]+)/delete', 'delete_thread', name='delete_thread'), - url(r'threads/(?P[\w\-]+)/upvote$', 'vote_for_thread', {'value': 'up'}, name='upvote_thread'), - url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', {'value': 'down'}, name='downvote_thread'), - url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', name='flag_abuse_for_thread'), - url(r'threads/(?P[\w\-]+)/unFlagAbuse$', 'un_flag_abuse_for_thread', name='un_flag_abuse_for_thread'), - url(r'threads/(?P[\w\-]+)/unvote$', 'undo_vote_for_thread', name='undo_vote_for_thread'), - url(r'threads/(?P[\w\-]+)/pin$', 'pin_thread', name='pin_thread'), - url(r'threads/(?P[\w\-]+)/unpin$', 'un_pin_thread', name='un_pin_thread'), - url(r'threads/(?P[\w\-]+)/follow$', 'follow_thread', name='follow_thread'), - url(r'threads/(?P[\w\-]+)/unfollow$', 'unfollow_thread', name='unfollow_thread'), - url(r'threads/(?P[\w\-]+)/close$', 'openclose_thread', name='openclose_thread'), - url(r'comments/(?P[\w\-]+)/update$', 'update_comment', name='update_comment'), - url(r'comments/(?P[\w\-]+)/endorse$', 'endorse_comment', name='endorse_comment'), - url(r'comments/(?P[\w\-]+)/reply$', 'create_sub_comment', name='create_sub_comment'), - url(r'comments/(?P[\w\-]+)/delete$', 'delete_comment', name='delete_comment'), - url(r'comments/(?P[\w\-]+)/upvote$', 'vote_for_comment', {'value': 'up'}, name='upvote_comment'), - url(r'comments/(?P[\w\-]+)/downvote$', 'vote_for_comment', {'value': 'down'}, name='downvote_comment'), - url(r'comments/(?P[\w\-]+)/unvote$', 'undo_vote_for_comment', name='undo_vote_for_comment'), - url(r'comments/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_comment', name='flag_abuse_for_comment'), - url(r'comments/(?P[\w\-]+)/unFlagAbuse$', 'un_flag_abuse_for_comment', name='un_flag_abuse_for_comment'), - url(r'^(?P[\w\-.]+)/threads/create$', 'create_thread', name='create_thread'), - url(r'^(?P[\w\-.]+)/follow$', 'follow_commentable', name='follow_commentable'), - url(r'^(?P[\w\-.]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'), - url(r'users$', 'users', name='users'), +urlpatterns = patterns( + 'django_comment_client.base.views', + url( + r'upload$', + 'upload', + name='upload', + ), + url( + r'threads/(?P[\w\-]+)/update$', + 'update_thread', + name='update_thread', + ), + url( + r'threads/(?P[\w\-]+)/reply$', + 'create_comment', + name='create_comment', + ), + url( + r'threads/(?P[\w\-]+)/delete', + 'delete_thread', + name='delete_thread', + ), + url( + r'threads/(?P[\w\-]+)/upvote$', + 'vote_for_thread', + { + 'value': 'up', + }, + name='upvote_thread', + ), + url( + r'threads/(?P[\w\-]+)/downvote$', + 'vote_for_thread', + { + 'value': 'down', + }, + name='downvote_thread', + ), + url( + r'threads/(?P[\w\-]+)/flagAbuse$', + 'flag_abuse_for_thread', + name='flag_abuse_for_thread', + ), + url( + r'threads/(?P[\w\-]+)/unFlagAbuse$', + 'un_flag_abuse_for_thread', + name='un_flag_abuse_for_thread', + ), + url( + r'threads/(?P[\w\-]+)/unvote$', + 'undo_vote_for_thread', + name='undo_vote_for_thread', + ), + url( + r'threads/(?P[\w\-]+)/pin$', + 'pin_thread', + name='pin_thread', + ), + url( + r'threads/(?P[\w\-]+)/unpin$', + 'un_pin_thread', + name='un_pin_thread', + ), + url( + r'threads/(?P[\w\-]+)/follow$', + 'follow_thread', + name='follow_thread', + ), + url( + r'threads/(?P[\w\-]+)/unfollow$', + 'unfollow_thread', + name='unfollow_thread', + ), + url( + r'threads/(?P[\w\-]+)/close$', + 'openclose_thread', + name='openclose_thread', + ), + url( + r'comments/(?P[\w\-]+)/update$', + 'update_comment', + name='update_comment', + ), + url( + r'comments/(?P[\w\-]+)/endorse$', + 'endorse_comment', + name='endorse_comment', + ), + url( + r'comments/(?P[\w\-]+)/reply$', + 'create_sub_comment', + name='create_sub_comment', + ), + url( + r'comments/(?P[\w\-]+)/delete$', + 'delete_comment', + name='delete_comment', + ), + url( + r'comments/(?P[\w\-]+)/upvote$', + 'vote_for_comment', + { + 'value': 'up', + }, + name='upvote_comment', + ), + url( + r'comments/(?P[\w\-]+)/downvote$', + 'vote_for_comment', + { + 'value': 'down', + }, + name='downvote_comment', + ), + url( + r'comments/(?P[\w\-]+)/unvote$', + 'undo_vote_for_comment', + name='undo_vote_for_comment', + ), + url( + r'comments/(?P[\w\-]+)/flagAbuse$', + 'flag_abuse_for_comment', + name='flag_abuse_for_comment', + ), + url( + r'comments/(?P[\w\-]+)/unFlagAbuse$', + 'un_flag_abuse_for_comment', + name='un_flag_abuse_for_comment', + ), + url( + r'^(?P[\w\-.]+)/threads/create$', + 'create_thread', + name='create_thread', + ), + url( + r'^(?P[\w\-.]+)/follow$', + 'follow_commentable', + name='follow_commentable', + ), + url( + r'^(?P[\w\-.]+)/unfollow$', + 'unfollow_commentable', + name='unfollow_commentable', + ), + url( + r'users$', + 'users', + name='users', + ), ) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 2a158f84e19b..de109a21b627 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -1,6 +1,5 @@ import functools import logging -import os.path import random import time import urlparse diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 8e2ec25ed433..b3b839ffad06 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -22,8 +22,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, - TEST_DATA_MOCK_MODULESTORE, - TEST_DATA_MONGO_MODULESTORE + TEST_DATA_MONGO_MODULESTORE, ) from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, ItemFactory @@ -128,11 +127,11 @@ def make_mock_thread_data(text, thread_id, num_children, group_id=None, group_na def make_mock_request_impl( - text, - thread_id="dummy_thread_id", - group_id=None, - commentable_id=None, - num_thread_responses=1, + text, + thread_id='dummy_thread_id', + group_id=None, + commentable_id=None, + num_thread_responses=1, ): def mock_request_impl(*args, **kwargs): url = args[1] @@ -947,7 +946,7 @@ def test_404_profiled_user(self, mock_request): request = RequestFactory().get("dummy_url") request.user = self.student with self.assertRaises(Http404): - response = views.user_profile( + views.user_profile( request, self.course.id.to_deprecated_string(), -999 @@ -957,7 +956,7 @@ def test_404_course(self, mock_request): request = RequestFactory().get("dummy_url") request.user = self.student with self.assertRaises(Http404): - response = views.user_profile( + views.user_profile( request, "non/existent/course", self.profiled_user.id diff --git a/lms/djangoapps/django_comment_client/forum/urls.py b/lms/djangoapps/django_comment_client/forum/urls.py index 863267fde9b0..f49524c78de3 100644 --- a/lms/djangoapps/django_comment_client/forum/urls.py +++ b/lms/djangoapps/django_comment_client/forum/urls.py @@ -1,9 +1,30 @@ from django.conf.urls.defaults import url, patterns -urlpatterns = patterns('django_comment_client.forum.views', # nopep8 - url(r'users/(?P\w+)/followed$', 'followed_threads', name='followed_threads'), - url(r'users/(?P\w+)$', 'user_profile', name='user_profile'), - url(r'^(?P[\w\-.]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), - url(r'^(?P[\w\-.]+)/inline$', 'inline_discussion', name='inline_discussion'), - url(r'', 'forum_form_discussion', name='forum_form_discussion'), +urlpatterns = patterns( + 'django_comment_client.forum.views', + url( + r'users/(?P\w+)/followed$', + 'followed_threads', + name='followed_threads', + ), + url( + r'users/(?P\w+)$', + 'user_profile', + name='user_profile', + ), + url( + r'^(?P[\w\-.]+)/threads/(?P\w+)$', + 'single_thread', + name='single_thread', + ), + url( + r'^(?P[\w\-.]+)/inline$', + 'inline_discussion', + name='inline_discussion', + ), + url( + r'', + 'forum_form_discussion', + name='forum_form_discussion', + ), ) diff --git a/lms/djangoapps/django_comment_client/helpers.py b/lms/djangoapps/django_comment_client/helpers.py index c0f7d751e09c..2808c771d960 100644 --- a/lms/djangoapps/django_comment_client/helpers.py +++ b/lms/djangoapps/django_comment_client/helpers.py @@ -19,8 +19,11 @@ def template_id_from_file_name(file_name): def process_mako(template_content): return Template(template_content).render_unicode() - def make_script_tag(id, content): - return u"".format(id, content) + def make_script_tag(identifier, content): + return u"".format( + identifier, + content, + ) return u'\n'.join( make_script_tag(template_id_from_file_name(file_name), process_mako(read_file(file_name))) diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index 1ee08bcca364..4576a934b371 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -126,7 +126,10 @@ def test(user, per, operator="or"): def check_permissions_by_view(user, course_id, content, name): assert isinstance(course_id, CourseKey) try: - p = VIEW_PERMISSIONS[name] + permissions = VIEW_PERMISSIONS[name] except KeyError: - logging.warning("Permission for view named %s does not exist in permissions.py" % name) - return _check_conditions_permissions(user, p, course_id, content) + logging.warning( + "Permission for view named %s does not exist in permissions.py", + name, + ) + return _check_conditions_permissions(user, permissions, course_id, content) 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 ec120885ecfb..ec2a67c0cbd7 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 @@ -31,8 +31,10 @@ def do_POST(self): # Every good post has at least an API key if 'X-Edx-Api-Key' in self.headers: response = self.server._response_str - # Log the response - logger.debug("Comment Service: sending response %s" % json.dumps(response)) + logger.debug( + "Comment Service: sending response %s", + json.dumps(response), + ) # Send a response back to the client self.send_response(200) @@ -69,7 +71,10 @@ def do_PUT(self): if 'X-Edx-Api-Key' in self.headers: response = self.server._response_str # Log the response - logger.debug("Comment Service: sending response %s" % json.dumps(response)) + logger.debug( + "Comment Service: sending response %s", + json.dumps(response), + ) # Send a response back to the client self.send_response(200) @@ -90,14 +95,17 @@ class MockCommentServiceServer(HTTPServer): A mock Comment Service server that responds to POST requests to localhost. ''' - def __init__(self, port_num, - response={'username': 'new', 'external_id': 1}): + def __init__(self, port_num, response=None): ''' Initialize the mock Comment Service server instance. *port_num* is the localhost port to listen to *response* is a dictionary that will be JSON-serialized and sent in response to comment service requests. ''' + response = response or { + 'username': 'new', + 'external_id': 1, + } self._response_str = json.dumps(response) handler = MockCommentServiceRequestHandler diff --git a/lms/djangoapps/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py b/lms/djangoapps/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py index 985c8a41dc58..15e708c3bab4 100644 --- a/lms/djangoapps/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py +++ b/lms/djangoapps/django_comment_client/tests/mock_cs_server/test_mock_cs_server.py @@ -2,7 +2,7 @@ import threading import json import urllib2 -from mock_cs_server import MockCommentServiceServer +from django_comment_client.tests.mock_cs_server import MockCommentServiceServer from nose.plugins.skip import SkipTest diff --git a/lms/djangoapps/django_comment_client/tests/test_models.py b/lms/djangoapps/django_comment_client/tests/test_models.py index 41a012a618fe..7c63808edd46 100644 --- a/lms/djangoapps/django_comment_client/tests/test_models.py +++ b/lms/djangoapps/django_comment_client/tests/test_models.py @@ -2,7 +2,6 @@ Tests for the django comment client integration models """ from django.test.testcases import TestCase -from django.test.utils import override_settings from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE @@ -34,10 +33,6 @@ def setUp(self): self.TA_role_2 = models.Role.objects.get_or_create(name="Community TA", course_id=self.course_id_2)[0] - class Dummy(): - def render_template(): - pass - def test_has_permission(self): # Whenever you add a permission to student_role, # Roles with the same FORUM_ROLE in same class also receives the same diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 4f9e111244ca..00ac70d29765 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -5,11 +5,9 @@ from django.core.urlresolvers import reverse from django.test import TestCase -from django.test.utils import override_settings from edxmako import add_lookup import mock -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from django_comment_client.tests.factories import RoleFactory from django_comment_client.tests.unicode import UnicodeTestMixin import django_comment_client.utils as utils diff --git a/lms/djangoapps/django_comment_client/tests/utils.py b/lms/djangoapps/django_comment_client/tests/utils.py index c7822eea78d2..557e36c2023d 100644 --- a/lms/djangoapps/django_comment_client/tests/utils.py +++ b/lms/djangoapps/django_comment_client/tests/utils.py @@ -1,8 +1,6 @@ -from django.test.utils import override_settings from mock import patch from openedx.core.djangoapps.course_groups.models import CourseUserGroup -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from django_comment_common.models import Role from django_comment_common.utils import seed_permissions_roles from student.tests.factories import CourseEnrollmentFactory, UserFactory diff --git a/lms/djangoapps/django_comment_client/urls.py b/lms/djangoapps/django_comment_client/urls.py index 98700da4ab1f..9e87c00abb3a 100644 --- a/lms/djangoapps/django_comment_client/urls.py +++ b/lms/djangoapps/django_comment_client/urls.py @@ -1,6 +1,13 @@ from django.conf.urls.defaults import url, patterns, include -urlpatterns = patterns('', # nopep8 - url(r'forum/?', include('django_comment_client.forum.urls')), - url(r'', include('django_comment_client.base.urls')), +urlpatterns = patterns( + '', + url( + r'forum/?', + include('django_comment_client.forum.urls'), + ), + url( + r'', + include('django_comment_client.base.urls'), + ), ) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 3044f80535d1..9c43273530b6 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -65,7 +65,11 @@ def _get_discussion_modules(course): def has_required_keys(module): for key in ('discussion_id', 'discussion_category', 'discussion_target'): if getattr(module, key) is None: - log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location)) + log.warning( + "Required key '%s' not in discussion %s, leaving out of category map", + key, + module.location, + ) return False return True @@ -133,8 +137,6 @@ def _sort_map_entries(category_map, sort_alpha): def get_discussion_category_map(course): - course_id = course.id - unexpanded_category_map = defaultdict(list) modules = _get_discussion_modules(course) @@ -143,13 +145,18 @@ def get_discussion_category_map(course): cohorted_discussion_ids = course.cohorted_discussions for module in modules: - id = module.discussion_id + discussion_id = module.discussion_id title = module.discussion_target sort_key = module.sort_key category = " / ".join([x.strip() for x in module.discussion_category.split("/")]) #Handle case where module.start is None entry_start_date = module.start if module.start else datetime.max.replace(tzinfo=pytz.UTC) - unexpanded_category_map[category].append({"title": title, "id": id, "sort_key": sort_key, "start_date": entry_start_date}) + unexpanded_category_map[category].append({ + 'id': discussion_id, + 'sort_key': sort_key, + 'start_date': entry_start_date, + 'title': title, + }) category_map = {"entries": defaultdict(dict), "subcategories": defaultdict(dict)} for category_path, entries in unexpanded_category_map.items(): @@ -228,7 +235,8 @@ def __init__(self, data=None): class JsonError(HttpResponse): - def __init__(self, error_messages=[], status=400): + def __init__(self, error_messages=None, status=400): + error_messages = error_messages or [] if isinstance(error_messages, basestring): error_messages = [error_messages] content = simplejson.dumps({'errors': error_messages}, @@ -270,7 +278,11 @@ def process_response(self, request, response): query_time = query.get('duration', 0) / 1000 total_time += float(query_time) - log.info('%s queries run, total %s seconds' % (len(connection.queries), total_time)) + log.info( + "%s queries run, total %s seconds", + len(connection.queries), + total_time, + ) return response @@ -378,8 +390,13 @@ def add_courseware_context(content_list, course): location = id_map[commentable_id]["location"].to_deprecated_string() title = id_map[commentable_id]["title"] - url = reverse('jump_to', kwargs={"course_id": course.id.to_deprecated_string(), - "location": location}) + url = reverse( + 'jump_to', + kwargs={ + 'course_id': course.id.to_deprecated_string(), + 'location': location, + }, + ) content.update({"courseware_url": url, "courseware_title": title}) @@ -418,15 +435,21 @@ def prepare_content(content, course_key, is_staff=False): try: endorser = User.objects.get(pk=endorsement["user_id"]) except User.DoesNotExist: - log.error("User ID {0} in endorsement for comment {1} but not in our DB.".format( + log.error( + "User ID %s in endorsement for comment %s but not in our DB.", content.get('user_id'), - content.get('id')) + content.get('id'), ) # Only reveal endorser if requester can see author or if endorser is staff if ( - endorser and - ("username" in fields or cached_has_permission(endorser, "endorse_comment", course_key)) + endorser + and + ( + "username" in fields + or + cached_has_permission(endorser, "endorse_comment", course_key) + ) ): endorsement["username"] = endorser.username else: diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 1b41e2664180..208d025c02bf 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -11,7 +11,6 @@ from edxnotes.decorators import edxnotes from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable from django.conf import settings -from django.test import TestCase from django.core.urlresolvers import reverse from django.core.exceptions import ImproperlyConfigured from oauth2_provider.tests.factories import ClientFactory diff --git a/lms/djangoapps/foldit/models.py b/lms/djangoapps/foldit/models.py index 4f0b3d16f611..703894a0adbe 100644 --- a/lms/djangoapps/foldit/models.py +++ b/lms/djangoapps/foldit/models.py @@ -37,7 +37,7 @@ def display_score(score, sum_of=1): return (-score) * 10 + 8000 * sum_of @staticmethod - def get_tops_n(n, puzzles=['994559'], course_list=None): + def get_tops_n(n, puzzles=None, course_list=None): """ Arguments: puzzles: a list of puzzle ids that we will use. If not specified, @@ -54,6 +54,9 @@ def get_tops_n(n, puzzles=['994559'], course_list=None): score: 12000} ...] """ + puzzles = puzzles or [ + '994559', + ] if not isinstance(puzzles, list): puzzles = [puzzles] if course_list is None: @@ -83,7 +86,7 @@ class PuzzleComplete(models.Model): e.g. PuzzleID 1234, set 1, subset 3. (Sets and subsets correspond to levels in the intro puzzles) """ - class Meta: + class Meta(object): # there should only be one puzzle complete entry for any particular # puzzle for any user unique_together = ('user', 'puzzle_id', 'puzzle_set', 'puzzle_subset') diff --git a/lms/djangoapps/instructor/management/commands/compute_grades.py b/lms/djangoapps/instructor/management/commands/compute_grades.py index 6d0d45d548cc..c5bdbfe6cfe2 100644 --- a/lms/djangoapps/instructor/management/commands/compute_grades.py +++ b/lms/djangoapps/instructor/management/commands/compute_grades.py @@ -5,7 +5,6 @@ """ from instructor.offline_gradecalc import offline_grade_calculation from courseware.courses import get_course_by_id -from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey diff --git a/lms/djangoapps/instructor/management/commands/dump_grades.py b/lms/djangoapps/instructor/management/commands/dump_grades.py index 2270e71dcff2..a0a1dcea18c7 100644 --- a/lms/djangoapps/instructor/management/commands/dump_grades.py +++ b/lms/djangoapps/instructor/management/commands/dump_grades.py @@ -10,7 +10,6 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.modulestore.django import modulestore from django.core.management.base import BaseCommand from instructor.utils import DummyRequest diff --git a/lms/djangoapps/instructor/management/tests/test_openended_commands.py b/lms/djangoapps/instructor/management/tests/test_openended_commands.py index b2f2449a5a91..2348ef28bf18 100644 --- a/lms/djangoapps/instructor/management/tests/test_openended_commands.py +++ b/lms/djangoapps/instructor/management/tests/test_openended_commands.py @@ -6,7 +6,6 @@ from pytz import UTC from django.conf import settings -from django.test.utils import override_settings from opaque_keys.edx.locations import Location import capa.xqueue_interface as xqueue_interface @@ -14,7 +13,6 @@ from courseware.tests.factories import StudentModuleFactory, UserFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from xmodule.modulestore.xml_importer import import_from_xml from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild from xmodule.tests.test_util_open_ended import ( diff --git a/lms/djangoapps/instructor/tests/test_access.py b/lms/djangoapps/instructor/tests/test_access.py index 817077c1b862..a22fe7f1eed1 100644 --- a/lms/djangoapps/instructor/tests/test_access.py +++ b/lms/djangoapps/instructor/tests/test_access.py @@ -7,8 +7,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from django.test.utils import override_settings -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.roles import CourseBetaTesterRole, CourseStaffRole from django_comment_common.models import (Role, diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 9a2dbbc87607..106366dfabc2 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -31,7 +31,6 @@ from course_modes.models import CourseMode from courseware.models import StudentModule from courseware.tests.factories import StaffFactory, InstructorFactory, BetaTesterFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from courseware.tests.helpers import LoginEnrollmentTestCase from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA from django_comment_common.utils import seed_permissions_roles diff --git a/lms/djangoapps/instructor/tests/test_ecommerce.py b/lms/djangoapps/instructor/tests/test_ecommerce.py index 6aaf5c5f8a95..62bce21257b9 100644 --- a/lms/djangoapps/instructor/tests/test_ecommerce.py +++ b/lms/djangoapps/instructor/tests/test_ecommerce.py @@ -5,13 +5,14 @@ from django.core.urlresolvers import reverse import datetime import pytz -from django.test.utils import override_settings from mock import patch from course_modes.models import CourseMode -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.roles import CourseFinanceAdminRole -from shoppingcart.models import Coupon, PaidCourseRegistration, CourseRegistrationCode +from shoppingcart.models import ( + Coupon, + CourseRegistrationCode, +) from student.tests.factories import AdminFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory diff --git a/lms/djangoapps/instructor/tests/test_email.py b/lms/djangoapps/instructor/tests/test_email.py index f3ae5648f201..803cbf1ead33 100644 --- a/lms/djangoapps/instructor/tests/test_email.py +++ b/lms/djangoapps/instructor/tests/test_email.py @@ -6,7 +6,6 @@ """ from django.conf import settings from django.core.urlresolvers import reverse -from django.test.utils import override_settings from mock import patch from opaque_keys.edx.locations import SlashSeparatedCourseKey diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 7f340d779a37..fd36c3700016 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -9,12 +9,10 @@ from courseware.models import StudentModule from django.conf import settings from django.test import TestCase -from django.test.utils import override_settings from django.utils.translation import get_language from django.utils.translation import override as override_language from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.models import CourseEnrollment, CourseEnrollmentAllowed from instructor.enrollment import ( diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py index 325b35ec1622..acbe95d6f4d6 100644 --- a/lms/djangoapps/instructor/tests/test_hint_manager.py +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -1,12 +1,10 @@ import json from django.test.client import Client, RequestFactory -from django.test.utils import override_settings from mock import patch, MagicMock from courseware.models import XModuleUserStateSummaryField from courseware.tests.factories import UserStateSummaryFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE import instructor.hint_manager as view from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -111,10 +109,32 @@ def test_gethints_other(self): out = view.get_hints(post, self.course_id, 'hints') print out self.assertTrue(out['other_field'] == 'mod_queue') - expected = {self.problem_id: [('1.0', {'1': ['Hint 1', 2], - '3': ['Hint 3', 12]}), - ('2.0', {'4': ['Hint 4', 3]}) - ]} + expected = { + self.problem_id: [ + ( + '1.0', + { + '1': [ + 'Hint 1', + 2, + ], + '3': [ + 'Hint 3', + 12, + ], + }, + ), + ( + '2.0', + { + '4': [ + 'Hint 4', + 3, + ], + }, + ), + ], + } self.assertTrue(out['all_hints'] == expected) def test_deletehints(self): diff --git a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py b/lms/djangoapps/instructor/tests/test_legacy_enrollment.py index 0eb3efecee2b..3eea58c28d0f 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_legacy_enrollment.py @@ -6,11 +6,9 @@ import ddt from mock import patch -from django.test.utils import override_settings from django.contrib.auth.models import User from django.core.urlresolvers import reverse from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory, AdminFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -188,7 +186,7 @@ def test_get_and_clean_student_list(self): """ string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com \n mno@test.com " - cleaned_string, cleaned_string_lc = get_and_clean_student_list(string) + cleaned_string, _cleaned_string_lc = get_and_clean_student_list(string) self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com', 'mno@test.com']) @ddt.data('http', 'https') diff --git a/lms/djangoapps/instructor/tests/test_legacy_xss.py b/lms/djangoapps/instructor/tests/test_legacy_xss.py index dec53ae9af0e..bfdd404e25d5 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_xss.py +++ b/lms/djangoapps/instructor/tests/test_legacy_xss.py @@ -4,10 +4,8 @@ from django.conf import settings from django.test.client import RequestFactory -from django.test.utils import override_settings from markupsafe import escape -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory, CourseEnrollmentFactory from edxmako.tests import mako_middleware_process_request from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/instructor/tests/test_spoc_gradebook.py b/lms/djangoapps/instructor/tests/test_spoc_gradebook.py index f39400457d81..ccb5088a0e19 100644 --- a/lms/djangoapps/instructor/tests/test_spoc_gradebook.py +++ b/lms/djangoapps/instructor/tests/test_spoc_gradebook.py @@ -2,12 +2,10 @@ Tests of the instructor dashboard spoc gradebook """ -from django.test.utils import override_settings from django.core.urlresolvers import reverse from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory, AdminFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from capa.tests.response_xml_factory import StringResponseXMLFactory from courseware.tests.factories import StudentModuleFactory from xmodule.modulestore.django import modulestore diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 0fdc2534de54..9724eb7bcdf2 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -8,11 +8,9 @@ import json import unittest -from django.test.utils import override_settings from django.utils.timezone import utc from courseware.models import StudentModule -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory from xmodule.fields import Date from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -148,7 +146,7 @@ def setUp(self): week1 = ItemFactory.create(due=due, parent=course) week2 = ItemFactory.create(due=due, parent=course) - homework = ItemFactory.create( + ItemFactory.create( parent=week1, due=due ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 5e923bfe7620..acf2c916a5a9 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -31,8 +31,6 @@ from student import auth from student.roles import CourseSalesAdminRole from util.file import store_uploaded_file, course_and_time_based_filename_generator, FileValidationException, UniversalNewlineIterator -import datetime -import pytz from util.json_request import JsonResponse from instructor.views.instructor_task_helpers import extract_email_features, extract_task_features diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 328190cdae94..20055ce15ba6 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -4,87 +4,196 @@ from django.conf.urls import patterns, url -urlpatterns = patterns('', # nopep8 - url(r'^students_update_enrollment$', - 'instructor.views.api.students_update_enrollment', name="students_update_enrollment"), - url(r'^register_and_enroll_students$', - 'instructor.views.api.register_and_enroll_students', name="register_and_enroll_students"), - url(r'^list_course_role_members$', - 'instructor.views.api.list_course_role_members', name="list_course_role_members"), - url(r'^modify_access$', - 'instructor.views.api.modify_access', name="modify_access"), - url(r'^bulk_beta_modify_access$', - 'instructor.views.api.bulk_beta_modify_access', name="bulk_beta_modify_access"), - url(r'^get_grading_config$', - 'instructor.views.api.get_grading_config', name="get_grading_config"), - url(r'^get_students_features(?P/csv)?$', - 'instructor.views.api.get_students_features', name="get_students_features"), - url(r'^get_user_invoice_preference$', - 'instructor.views.api.get_user_invoice_preference', name="get_user_invoice_preference"), - url(r'^get_sale_records(?P/csv)?$', - 'instructor.views.api.get_sale_records', name="get_sale_records"), - url(r'^get_sale_order_records$', - 'instructor.views.api.get_sale_order_records', name="get_sale_order_records"), - url(r'^sale_validation_url$', - 'instructor.views.api.sale_validation', name="sale_validation"), - url(r'^get_anon_ids$', - 'instructor.views.api.get_anon_ids', name="get_anon_ids"), - url(r'^get_distribution$', - 'instructor.views.api.get_distribution', name="get_distribution"), - url(r'^get_student_progress_url$', - 'instructor.views.api.get_student_progress_url', name="get_student_progress_url"), - url(r'^reset_student_attempts$', - 'instructor.views.api.reset_student_attempts', name="reset_student_attempts"), - url(r'^rescore_problem$', - 'instructor.views.api.rescore_problem', name="rescore_problem"), - url(r'^list_instructor_tasks$', - 'instructor.views.api.list_instructor_tasks', name="list_instructor_tasks"), - url(r'^list_background_email_tasks$', - 'instructor.views.api.list_background_email_tasks', name="list_background_email_tasks"), - url(r'^list_email_content$', - 'instructor.views.api.list_email_content', name="list_email_content"), - url(r'^list_forum_members$', - 'instructor.views.api.list_forum_members', name="list_forum_members"), - url(r'^update_forum_role_membership$', - 'instructor.views.api.update_forum_role_membership', name="update_forum_role_membership"), - url(r'^proxy_legacy_analytics$', - 'instructor.views.api.proxy_legacy_analytics', name="proxy_legacy_analytics"), - url(r'^send_email$', - 'instructor.views.api.send_email', name="send_email"), - url(r'^change_due_date$', 'instructor.views.api.change_due_date', - name='change_due_date'), - url(r'^reset_due_date$', 'instructor.views.api.reset_due_date', - name='reset_due_date'), - url(r'^show_unit_extensions$', 'instructor.views.api.show_unit_extensions', - name='show_unit_extensions'), - url(r'^show_student_extensions$', 'instructor.views.api.show_student_extensions', - name='show_student_extensions'), +urlpatterns = patterns( + '', + url( + r'^students_update_enrollment$', + 'instructor.views.api.students_update_enrollment', + name='students_update_enrollment', + ), + url( + r'^register_and_enroll_students$', + 'instructor.views.api.register_and_enroll_students', + name='register_and_enroll_students', + ), + url( + r'^list_course_role_members$', + 'instructor.views.api.list_course_role_members', + name='list_course_role_members', + ), + url( + r'^modify_access$', + 'instructor.views.api.modify_access', + name='modify_access', + ), + url( + r'^bulk_beta_modify_access$', + 'instructor.views.api.bulk_beta_modify_access', + name='bulk_beta_modify_access', + ), + url( + r'^get_grading_config$', + 'instructor.views.api.get_grading_config', + name='get_grading_config', + ), + url( + r'^get_students_features(?P/csv)?$', + 'instructor.views.api.get_students_features', + name='get_students_features', + ), + url( + r'^get_user_invoice_preference$', + 'instructor.views.api.get_user_invoice_preference', + name='get_user_invoice_preference', + ), + url( + r'^get_sale_records(?P/csv)?$', + 'instructor.views.api.get_sale_records', + name='get_sale_records', + ), + url( + r'^get_sale_order_records$', + 'instructor.views.api.get_sale_order_records', + name='get_sale_order_records', + ), + url( + r'^sale_validation_url$', + 'instructor.views.api.sale_validation', + name='sale_validation', + ), + url( + r'^get_anon_ids$', + 'instructor.views.api.get_anon_ids', + name='get_anon_ids', + ), + url( + r'^get_distribution$', + 'instructor.views.api.get_distribution', + name='get_distribution', + ), + url( + r'^get_student_progress_url$', + 'instructor.views.api.get_student_progress_url', + name='get_student_progress_url', + ), + url( + r'^reset_student_attempts$', + 'instructor.views.api.reset_student_attempts', + name='reset_student_attempts', + ), + url( + r'^rescore_problem$', + 'instructor.views.api.rescore_problem', + name='rescore_problem', + ), + url( + r'^list_instructor_tasks$', + 'instructor.views.api.list_instructor_tasks', + name='list_instructor_tasks', + ), + url( + r'^list_background_email_tasks$', + 'instructor.views.api.list_background_email_tasks', + name='list_background_email_tasks', + ), + url( + r'^list_email_content$', + 'instructor.views.api.list_email_content', + name='list_email_content', + ), + url( + r'^list_forum_members$', + 'instructor.views.api.list_forum_members', + name='list_forum_members', + ), + url( + r'^update_forum_role_membership$', + 'instructor.views.api.update_forum_role_membership', + name='update_forum_role_membership', + ), + url( + r'^proxy_legacy_analytics$', + 'instructor.views.api.proxy_legacy_analytics', + name='proxy_legacy_analytics', + ), + url( + r'^send_email$', + 'instructor.views.api.send_email', + name='send_email', + ), + url( + r'^change_due_date$', + 'instructor.views.api.change_due_date', + name='change_due_date', + ), + url( + r'^reset_due_date$', + 'instructor.views.api.reset_due_date', + name='reset_due_date', + ), + url( + r'^show_unit_extensions$', + 'instructor.views.api.show_unit_extensions', + name='show_unit_extensions', + ), + url( + r'^show_student_extensions$', + 'instructor.views.api.show_student_extensions', + name='show_student_extensions', + ), # Grade downloads... - url(r'^list_report_downloads$', - 'instructor.views.api.list_report_downloads', name="list_report_downloads"), - url(r'calculate_grades_csv$', - 'instructor.views.api.calculate_grades_csv', name="calculate_grades_csv"), + url( + r'^list_report_downloads$', + 'instructor.views.api.list_report_downloads', + name='list_report_downloads', + ), + url( + r'calculate_grades_csv$', + 'instructor.views.api.calculate_grades_csv', + name='calculate_grades_csv', + ), # Registration Codes.. - url(r'get_registration_codes$', - 'instructor.views.api.get_registration_codes', name="get_registration_codes"), - url(r'generate_registration_codes$', - 'instructor.views.api.generate_registration_codes', name="generate_registration_codes"), - url(r'active_registration_codes$', - 'instructor.views.api.active_registration_codes', name="active_registration_codes"), - url(r'spent_registration_codes$', - 'instructor.views.api.spent_registration_codes', name="spent_registration_codes"), + url( + r'get_registration_codes$', + 'instructor.views.api.get_registration_codes', + name='get_registration_codes', + ), + url( + r'generate_registration_codes$', + 'instructor.views.api.generate_registration_codes', + name='generate_registration_codes', + ), + url( + r'active_registration_codes$', + 'instructor.views.api.active_registration_codes', + name='active_registration_codes', + ), + url( + r'spent_registration_codes$', + 'instructor.views.api.spent_registration_codes', + name='spent_registration_codes', + ), # Coupon Codes.. - url(r'get_coupon_codes', - 'instructor.views.api.get_coupon_codes', name="get_coupon_codes"), + url( + r'get_coupon_codes', + 'instructor.views.api.get_coupon_codes', + name='get_coupon_codes', + ), # spoc gradebook - url(r'^gradebook$', - 'instructor.views.api.spoc_gradebook', name='spoc_gradebook'), + url( + r'^gradebook$', + 'instructor.views.api.spoc_gradebook', + name='spoc_gradebook', + ), # Cohort management - url(r'add_users_to_cohorts$', - 'instructor.views.api.add_users_to_cohorts', name="add_users_to_cohorts"), + url( + r'add_users_to_cohorts$', + 'instructor.views.api.add_users_to_cohorts', + name='add_users_to_cohorts', + ), ) diff --git a/lms/djangoapps/instructor/views/coupons.py b/lms/djangoapps/instructor/views/coupons.py index 3abfc2c3bb37..435471e23b24 100644 --- a/lms/djangoapps/instructor/views/coupons.py +++ b/lms/djangoapps/instructor/views/coupons.py @@ -3,11 +3,9 @@ """ from django.contrib.auth.decorators import login_required from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Q from django.views.decorators.http import require_POST from django.utils.translation import ugettext as _ from util.json_request import JsonResponse -from django.http import HttpResponse, HttpResponseNotFound from shoppingcart.models import Coupon, CourseRegistrationCode from opaque_keys.edx.locations import SlashSeparatedCourseKey import datetime diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index 6681e3737c8b..6f0318443088 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -5,8 +5,11 @@ """ import json from shoppingcart.models import ( - PaidCourseRegistration, CouponRedemption, Invoice, CourseRegCodeItem, - OrderTypes, RegistrationCodeRedemption, CourseRegistrationCode, CourseRegistrationCodeInvoiceItem + PaidCourseRegistration, + CouponRedemption, + CourseRegCodeItem, + RegistrationCodeRedemption, + CourseRegistrationCodeInvoiceItem, ) from django.db.models import Q from django.conf import settings diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 344ef2dc43af..97ed430617b1 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -89,16 +89,18 @@ def _get_xmodule_instance_args(request, task_id): permit old-style xqueue callbacks directly to the appropriate module in the LMS. The `task_id` is also passed to the tracking log function. """ - request_info = {'username': request.user.username, - 'ip': request.META['REMOTE_ADDR'], - 'agent': request.META.get('HTTP_USER_AGENT', ''), - 'host': request.META['SERVER_NAME'], - } - - xmodule_instance_args = {'xqueue_callback_url_prefix': get_xqueue_callback_url_prefix(request), - 'request_info': request_info, - 'task_id': task_id, - } + request_info = { + 'username': request.user.username, + 'ip': request.META['REMOTE_ADDR'], + 'agent': request.META.get('HTTP_USER_AGENT', ''), + 'host': request.META['SERVER_NAME'], + } + + xmodule_instance_args = { + 'xqueue_callback_url_prefix': get_xqueue_callback_url_prefix(request), + 'request_info': request_info, + 'task_id': task_id, + } return xmodule_instance_args diff --git a/lms/djangoapps/instructor_task/subtasks.py b/lms/djangoapps/instructor_task/subtasks.py index 093eddce9818..3dddaac6b6c3 100644 --- a/lms/djangoapps/instructor_task/subtasks.py +++ b/lms/djangoapps/instructor_task/subtasks.py @@ -4,7 +4,6 @@ from time import time import json from uuid import uuid4 -import math import psutil from contextlib import contextmanager @@ -71,12 +70,12 @@ def track_memory_usage(metric, course_id): def _generate_items_for_subtask( - item_queryset, - item_fields, - total_num_items, - items_per_task, - total_num_subtasks, - course_id, + item_queryset, + item_fields, + total_num_items, + items_per_task, + total_num_subtasks, + course_id, ): """ Generates a chunk of "items" that should be passed into a subtask. @@ -165,7 +164,7 @@ def __init__(self, task_id, attempted=None, succeeded=0, failed=0, skipped=0, re self.state = state if state is not None else QUEUING @classmethod - def from_dict(self, d): + def from_dict(cls, d): """Construct a SubtaskStatus object from a dict representation.""" options = dict(d) task_id = options['task_id'] @@ -173,9 +172,9 @@ def from_dict(self, d): return SubtaskStatus.create(task_id, **options) @classmethod - def create(self, task_id, **options): + def create(cls, task_id, **options): """Construct a SubtaskStatus object.""" - return self(task_id, **options) + return cls(task_id, **options) def to_dict(self): """ diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index c071ba8127d1..c55e9ed706ef 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -82,7 +82,6 @@ def setUp(self): def test_submit_nonexistent_modules(self): # confirm that a rescore of a non-existent module returns an exception problem_url = InstructorTaskModuleTestCase.problem_location("NonexistentProblem") - course_id = self.course.id request = None with self.assertRaises(ItemNotFoundError): submit_rescore_problem_for_student(request, problem_url, self.student) @@ -98,7 +97,6 @@ def test_submit_nonrescorable_modules(self): # (Note that it is easier to test a scoreable but non-rescorable module in test_tasks, # where we are creating real modules.) problem_url = self.problem_section.location - course_id = self.course.id request = None with self.assertRaises(NotImplementedError): submit_rescore_problem_for_student(request, problem_url, self.student) diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index e738ea7be57d..7e8d67c7214d 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/lms/djangoapps/instructor_task/tests/test_base.py @@ -13,12 +13,10 @@ from django.conf import settings from django.test.testcases import TestCase from django.contrib.auth.models import User -from django.test.utils import override_settings from opaque_keys.edx.locations import Location, SlashSeparatedCourseKey from capa.tests.response_xml_factory import OptionResponseXMLFactory from courseware.model_data import StudentModule -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from courseware.tests.tests import LoginEnrollmentTestCase from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore import ModuleStoreEnum @@ -81,9 +79,10 @@ def _create_entry(self, task_state=QUEUING, task_output=None, student=None): def _create_failure_entry(self): """Creates a InstructorTask entry representing a failed task.""" # view task entry for task failure - progress = {'message': TEST_FAILURE_MESSAGE, - 'exception': TEST_FAILURE_EXCEPTION, - } + progress = { + 'message': TEST_FAILURE_MESSAGE, + 'exception': TEST_FAILURE_EXCEPTION, + } return self._create_entry(task_state=FAILURE, task_output=progress) def _create_success_entry(self, student=None): @@ -92,11 +91,12 @@ def _create_success_entry(self, student=None): def _create_progress_entry(self, student=None, task_state=PROGRESS): """Creates a InstructorTask entry representing a task in progress.""" - progress = {'attempted': 3, - 'succeeded': 2, - 'total': 5, - 'action_name': 'rescored', - } + progress = { + 'attempted': 3, + 'succeeded': 2, + 'total': 5, + 'action_name': 'rescored', + } return self._create_entry(task_state=task_state, task_output=progress, student=student) @@ -232,11 +232,12 @@ def redefine_option_problem(self, problem_url_name): def get_student_module(self, username, descriptor): """Get StudentModule object for test course, given the `username` and the problem's `descriptor`.""" - return StudentModule.objects.get(course_id=self.course.id, - student=User.objects.get(username=username), - module_type=descriptor.location.category, - module_state_key=descriptor.location, - ) + return StudentModule.objects.get( + course_id=self.course.id, + student=User.objects.get(username=username), + module_type=descriptor.location.category, + module_state_key=descriptor.location, + ) class TestReportMixin(object): diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index d6e2ce8cac8e..48c8b1cd1e9d 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -61,9 +61,11 @@ def _get_xmodule_instance_args(self): """ Calculate dummy values for parameters needed for instantiating xmodule instances. """ - return {'xqueue_callback_url_prefix': 'dummy_value', - 'request_info': {}, - } + return { + 'xqueue_callback_url_prefix': 'dummy_value', + 'request_info': { + }, + } def _run_task_with_mock_celery(self, task_class, entry_id, task_id, expected_failure_message=None): """Submit a task and mock how celery provides a current_task.""" diff --git a/lms/djangoapps/instructor_task/tests/test_views.py b/lms/djangoapps/instructor_task/tests/test_views.py index 5dd1e4fd1403..72dd7b62b0ae 100644 --- a/lms/djangoapps/instructor_task/tests/test_views.py +++ b/lms/djangoapps/instructor_task/tests/test_views.py @@ -115,12 +115,13 @@ def test_get_status_from_legacy_success(self): def _create_email_subtask_entry(self, total=5, attempted=3, succeeded=2, skipped=0, task_state=PROGRESS): """Create an InstructorTask with subtask defined and email argument.""" - progress = {'attempted': attempted, - 'succeeded': succeeded, - 'skipped': skipped, - 'total': total, - 'action_name': 'emailed', - } + progress = { + 'attempted': attempted, + 'succeeded': succeeded, + 'skipped': skipped, + 'total': total, + 'action_name': 'emailed', + } instructor_task = self._create_entry(task_state=task_state, task_output=progress) instructor_task.subtasks = {} instructor_task.task_input = json.dumps({'email_id': 134}) diff --git a/lms/djangoapps/licenses/management/commands/generate_serial_numbers.py b/lms/djangoapps/licenses/management/commands/generate_serial_numbers.py index 9774427b9c1b..7e935a831ce3 100644 --- a/lms/djangoapps/licenses/management/commands/generate_serial_numbers.py +++ b/lms/djangoapps/licenses/management/commands/generate_serial_numbers.py @@ -24,8 +24,6 @@ class Command(BaseCommand): args = "course_id software_id count" def handle(self, *args, **options): - """ - """ course_id, software_name, count = self._parse_arguments(args) software, _ = CourseSoftware.objects.get_or_create(course_id=course_id, diff --git a/lms/djangoapps/licenses/management/commands/import_serial_numbers.py b/lms/djangoapps/licenses/management/commands/import_serial_numbers.py index ed9ce03ca2d7..fbb93d3a287b 100644 --- a/lms/djangoapps/licenses/management/commands/import_serial_numbers.py +++ b/lms/djangoapps/licenses/management/commands/import_serial_numbers.py @@ -24,8 +24,6 @@ class Command(BaseCommand): args = "course_id software_id serial_file" def handle(self, *args, **options): - """ - """ course_id, software_name, filename = self._parse_arguments(args) software, _ = CourseSoftware.objects.get_or_create(course_id=course_id, diff --git a/lms/djangoapps/licenses/tests.py b/lms/djangoapps/licenses/tests.py index 0c37798b84d3..ff710607a66c 100644 --- a/lms/djangoapps/licenses/tests.py +++ b/lms/djangoapps/licenses/tests.py @@ -10,12 +10,10 @@ from django.test import TestCase from django.test.client import Client -from django.test.utils import override_settings from django.core.management import call_command from django.core.urlresolvers import reverse from nose.tools import assert_true # pylint: disable=no-name-in-module -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from licenses.models import CourseSoftware, UserLicense from student.tests.factories import UserFactory @@ -184,7 +182,7 @@ def test_import_serial_numbers(self): software_count = CourseSoftware.objects.all().count() self.assertEqual(2, software_count) - log.debug('Now we should have 3 sets of 20 serials'.format(size)) + log.debug('Now we should have 3 sets of {0} serials'.format(size)) licenses_count = UserLicense.objects.all().count() self.assertEqual(3 * size, licenses_count) diff --git a/lms/djangoapps/lms_migration/management/commands/create_groups.py b/lms/djangoapps/lms_migration/management/commands/create_groups.py index 6cdc0322782b..626f22c5d3c1 100644 --- a/lms/djangoapps/lms_migration/management/commands/create_groups.py +++ b/lms/djangoapps/lms_migration/management/commands/create_groups.py @@ -31,7 +31,7 @@ def create_groups(): cxfn = path(data_dir) / course_dir / 'course.xml' try: coursexml = etree.parse(cxfn) - except Exception as err: + except Exception: print "Oops, cannot read %s, skipping" % cxfn continue cxmlroot = coursexml.getroot() diff --git a/lms/djangoapps/lms_migration/management/commands/create_user.py b/lms/djangoapps/lms_migration/management/commands/create_user.py index 55590f6b2b33..e76ca1a1c6c4 100644 --- a/lms/djangoapps/lms_migration/management/commands/create_user.py +++ b/lms/djangoapps/lms_migration/management/commands/create_user.py @@ -44,7 +44,7 @@ def complete(self, text, state): def GenPasswd(length=8, chars=string.letters + string.digits): - return ''.join([choice(chars) for i in range(length)]) + return ''.join([choice(chars) for _i in range(length)]) #----------------------------------------------------------------------------- # main command @@ -78,7 +78,7 @@ def handle(self, *args, **options): # get name from kerberos try: kname = os.popen("finger %s | grep 'name:'" % email).read().strip().split('name: ')[1].strip() - except: + except Exception: kname = '' name = raw_input('Full name: [%s] ' % kname).strip() if name == '': diff --git a/lms/djangoapps/lms_migration/migrate.py b/lms/djangoapps/lms_migration/migrate.py index eb33a14773db..f17cab7d784f 100644 --- a/lms/djangoapps/lms_migration/migrate.py +++ b/lms/djangoapps/lms_migration/migrate.py @@ -77,7 +77,10 @@ def manage_modulestores(request, reload_dir=None, commit_id=None): else: html += 'Permission denied' html += "" - log.debug('request denied, ALLOWED_IPS=%s' % ALLOWED_IPS) + log.debug( + "request denied, ALLOWED_IPS=%s", + ALLOWED_IPS, + ) return HttpResponse(html, status=403) #---------------------------------------- @@ -90,17 +93,27 @@ def manage_modulestores(request, reload_dir=None, commit_id=None): # reloading based on commit_id is needed when running mutiple worker threads, # so that a given thread doesn't reload the same commit multiple times current_commit_id = get_commit_id(def_ms.courses[reload_dir]) - log.debug('commit_id="%s"' % commit_id) - log.debug('current_commit_id="%s"' % current_commit_id) + log.debug( + "commit_id='%s'", + commit_id, + ) + log.debug( + 'current_commit_id="%s"', + current_commit_id, + ) if (commit_id is not None) and (commit_id == current_commit_id): html += "

Already at commit id %s for %s

" % (commit_id, reload_dir) - track.views.server_track(request, - 'reload %s skipped already at %s (pid=%s)' % (reload_dir, - commit_id, - os.getpid(), - ), - {}, page='migrate') + track.views.server_track( + request, + 'reload %s skipped already at %s (pid=%s)' % ( + reload_dir, + commit_id, + os.getpid(), + ), + {}, + page='migrate', + ) else: html += '

Reloaded course directory "%s"

' % reload_dir def_ms.try_load_course(reload_dir) @@ -158,9 +171,18 @@ def manage_modulestores(request, reload_dir=None, commit_id=None): #---------------------------------------- - log.debug('_MODULESTORES=%s' % ms) - log.debug('courses=%s' % courses) - log.debug('def_ms=%s' % unicode(def_ms)) + log.debug( + "_MODULESTORES=%s", + ms, + ) + log.debug( + "courses=%s", + courses, + ) + log.debug( + "def_ms=%s", + unicode(def_ms), + ) html += "" return HttpResponse(html) @@ -189,7 +211,11 @@ def gitreload(request, reload_dir=None): else: html += 'Permission denied' html += "" - log.debug('request denied from %s, ALLOWED_IPS=%s' % (ip, ALLOWED_IPS)) + log.debug( + "request denied from %s, ALLOWED_IPS=%s", + ip, + ALLOWED_IPS, + ) return HttpResponse(html) #---------------------------------------- @@ -197,14 +223,26 @@ def gitreload(request, reload_dir=None): if reload_dir is None and 'payload' in request.POST: payload = request.POST['payload'] - log.debug("payload=%s" % payload) + log.debug( + "payload=%s", + payload, + ) gitargs = json.loads(payload) - log.debug("gitargs=%s" % gitargs) + log.debug( + "gitargs=%s", + gitargs, + ) reload_dir = gitargs['repository']['name'] - log.debug("github reload_dir=%s" % reload_dir) + log.debug( + "github reload_dir=%s", + reload_dir, + ) gdir = settings.DATA_DIR / reload_dir if not os.path.exists(gdir): - log.debug("====> ERROR in gitreload - no such directory %s" % reload_dir) + log.debug( + "====> ERROR in gitreload - no such directory %s", + reload_dir, + ) return HttpResponse('Error') cmd = "cd %s; git reset --hard HEAD; git clean -f -d; git pull origin; chmod g+w course.xml" % gdir log.debug(os.popen(cmd).read()) @@ -213,7 +251,11 @@ def gitreload(request, reload_dir=None): if gh: ghurl = '%s/%s' % (gh, reload_dir) r = requests.get(ghurl) - log.debug("GITRELOAD_HOOK to %s: %s" % (ghurl, r.text)) + log.debug( + "GITRELOAD_HOOK to %s: %s", + ghurl, + r.text, + ) #---------------------------------------- # reload course if specified diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 267378f67452..41261c191a06 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -65,7 +65,7 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): """ course = CourseField() - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring model = CourseEnrollment fields = ('created', 'mode', 'is_active', 'course') lookup_field = 'username' @@ -81,7 +81,7 @@ class UserSerializer(serializers.HyperlinkedModelSerializer): lookup_field='username' ) - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring model = User fields = ('id', 'username', 'email', 'name', 'course_enrollments') lookup_field = 'username' diff --git a/lms/djangoapps/notes/api.py b/lms/djangoapps/notes/api.py index 657e97f92ea9..ed7e816a8494 100644 --- a/lms/djangoapps/notes/api.py +++ b/lms/djangoapps/notes/api.py @@ -148,7 +148,7 @@ def create(request, course_key): return ApiResponse(http_response=response, data=None) -def read(request, course_key, note_id): # pylint: disable=unused-argument (course_key) +def read(request, course_key, note_id): # pylint: disable=unused-argument ''' Returns a single annotation object. ''' @@ -163,7 +163,7 @@ def read(request, course_key, note_id): # pylint: disable=unused-argument (cour return ApiResponse(http_response=HttpResponse(), data=note.as_dict()) -def update(request, course_key, note_id): # pylint: disable=unused-argument (course_key) +def update(request, course_key, note_id): # pylint: disable=unused-argument ''' Updates an annotation object and returns a 303 with the read location. ''' @@ -247,7 +247,7 @@ def search(request, course_key): return ApiResponse(http_response=HttpResponse(), data=result) -def root(request, course_key): # pylint: disable=unused-argument (course_key, request) +def root(request, course_key): # pylint: disable=unused-argument ''' Returns version information about the API. ''' diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py index 7aab06e1c80f..9f9e9bbecd00 100644 --- a/lms/djangoapps/notes/tests.py +++ b/lms/djangoapps/notes/tests.py @@ -90,7 +90,8 @@ def login(self, as_student=None): self.client.login(username=username, password=password) - def url(self, name, args={}): + def url(self, name, args=None): + args = args or {} args.update({'course_id': self.course_key.to_deprecated_string()}) return reverse(name, kwargs=args) @@ -316,9 +317,15 @@ def test_search_note_params(self): {'limit': 0, 'offset': 0, 'uri': invalid_uri, 'expected_rows': 0, 'expected_total': 0}] for test in tests: - params = dict([(k, str(test[k])) - for k in ('limit', 'offset', 'uri') - if k in test]) + params = { + key: str(test[key]) + for key in [ + 'limit', + 'offset', + 'uri', + ] + if key in test + } resp = self.client.get(self.url('notes_api_search'), params, content_type='application/json', diff --git a/lms/djangoapps/notes/urls.py b/lms/djangoapps/notes/urls.py index 6abe92253a02..f97b94ab471d 100644 --- a/lms/djangoapps/notes/urls.py +++ b/lms/djangoapps/notes/urls.py @@ -1,10 +1,38 @@ from django.conf.urls import patterns, url -id_regex = r"(?P[0-9A-Fa-f]+)" -urlpatterns = patterns('notes.api', - url(r'^api$', 'api_request', {'resource': 'root'}, name='notes_api_root'), - url(r'^api/annotations$', 'api_request', {'resource': 'notes'}, name='notes_api_notes'), - url(r'^api/annotations/' + id_regex + r'$', 'api_request', {'resource': 'note'}, name='notes_api_note'), - url(r'^api/search', 'api_request', {'resource': 'search'}, name='notes_api_search') - ) +urlpatterns = patterns( + 'notes.api', + url( + r'^api$', + 'api_request', + { + 'resource': 'root', + }, + name='notes_api_root', + ), + url( + r'^api/annotations$', + 'api_request', + { + 'resource': 'notes', + }, + name='notes_api_notes', + ), + url( + r'^api/annotations/(?P[0-9A-Fa-f]+)$', + 'api_request', + { + 'resource': 'note', + }, + name='notes_api_note', + ), + url( + r'^api/search', + 'api_request', + { + 'resource': 'search', + }, + name='notes_api_search', + ), +) diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py index 91e84b063cbe..19e556a66056 100644 --- a/lms/djangoapps/notification_prefs/tests.py +++ b/lms/djangoapps/notification_prefs/tests.py @@ -4,7 +4,7 @@ from django.core.exceptions import PermissionDenied from django.http import Http404 from django.test import TestCase -from django.test.client import Client, RequestFactory +from django.test.client import RequestFactory from django.test.utils import override_settings from mock import Mock, patch diff --git a/lms/djangoapps/notifier_api/serializers.py b/lms/djangoapps/notifier_api/serializers.py index a3877f1e5b70..1705111b4a95 100644 --- a/lms/djangoapps/notifier_api/serializers.py +++ b/lms/djangoapps/notifier_api/serializers.py @@ -63,7 +63,7 @@ def get_course_info(self, user): pass return ret - class Meta: + class Meta(object): model = User fields = ("id", "email", "name", "preferences", "course_info") read_only_fields = ("id", "email") diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 98773bfedb12..3129576d3ecc 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -7,8 +7,6 @@ from xmodule.open_ended_grading_classes import peer_grading_service from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService -from xmodule.modulestore.django import ModuleI18nService - from courseware.access import has_access from edxmako.shortcuts import render_to_string from student.models import unique_id_for_user @@ -46,7 +44,7 @@ def staff_grading_notifications(course, user): if notifications['success']: if notifications['staff_needs_to_grade']: pending_grading = True - except: + except Exception: #Non catastrophic error, so no real action notifications = {} #This is a dev_facing_error @@ -81,7 +79,7 @@ def peer_grading_notifications(course, user): if notifications['success']: if notifications['student_needs_to_peer_grade']: pending_grading = True - except: + except Exception: #Non catastrophic error, so no real action notifications = {} #This is a dev_facing_error @@ -145,7 +143,7 @@ def combined_notifications(course, user): if (notifications.get('staff_needs_to_grade') or notifications.get('student_needs_to_peer_grade')): pending_grading = True - except: + except Exception: #Non catastrophic error, so no real action #This is a dev_facing_error log.exception( @@ -192,7 +190,7 @@ def _get_value_from_cache(key_name): try: value = json.loads(value) success = True - except: + except Exception: pass return success, value diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py index 4830aa5b2029..d6a54c26d53d 100644 --- a/lms/djangoapps/open_ended_grading/staff_grading_service.py +++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py @@ -11,7 +11,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.open_ended_grading_classes.grading_service_module import GradingService, GradingServiceError -from xmodule.modulestore.django import ModuleI18nService from courseware.access import has_access from edxmako.shortcuts import render_to_string @@ -271,8 +270,8 @@ def get_next(request, course_id): return _err_response('Missing required keys {0}'.format( ', '.join(missing))) grader_id = unique_id_for_user(request.user) - p = request.POST - location = course_key.make_usage_key_from_deprecated_string(p['location']) + post = request.POST + location = course_key.make_usage_key_from_deprecated_string(post['location']) return HttpResponse(json.dumps(_get_next(course_key, grader_id, location)), mimetype="application/json") @@ -380,36 +379,38 @@ def save_grade(request, course_id): if request.method != 'POST': raise Http404 - p = request.POST + post = request.POST required = set(['score', 'feedback', 'submission_id', 'location', 'submission_flagged']) - skipped = 'skipped' in p + skipped = 'skipped' in post #If the instructor has skipped grading the submission, then there will not be any rubric scores. #Only add in the rubric scores if the instructor has not skipped. if not skipped: required.add('rubric_scores[]') - actual = set(p.keys()) + actual = set(post.keys()) missing = required - actual if len(missing) > 0: return _err_response('Missing required keys {0}'.format( ', '.join(missing))) - success, message = check_feedback_length(p) + success, message = check_feedback_length(post) if not success: return _err_response(message) grader_id = unique_id_for_user(request.user) - location = course_key.make_usage_key_from_deprecated_string(p['location']) + location = course_key.make_usage_key_from_deprecated_string(post['location']) try: - result = staff_grading_service().save_grade(course_key, - grader_id, - p['submission_id'], - p['score'], - p['feedback'], - skipped, - p.getlist('rubric_scores[]'), - p['submission_flagged']) + result = staff_grading_service().save_grade( + course_key, + grader_id, + post['submission_id'], + post['score'], + post['feedback'], + skipped, + post.getlist('rubric_scores[]'), + post['submission_flagged'], + ) except GradingServiceError: #This is a dev_facing_error log.exception( diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 35d0fd2a05e2..110a97c88ae9 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -11,7 +11,6 @@ from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.test import RequestFactory -from django.test.utils import override_settings from edxmako.shortcuts import render_to_string from edxmako.tests import mako_middleware_process_request from mock import MagicMock, patch, Mock @@ -29,7 +28,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MOCK_MODULESTORE, TEST_DATA_MIXED_TOY_MODULESTORE + TEST_DATA_MIXED_TOY_MODULESTORE ) from xmodule.modulestore.xml_importer import import_from_xml from xmodule.open_ended_grading_classes import peer_grading_service, controller_query_service @@ -313,9 +312,9 @@ def test_get_next_submission_success(self): def test_get_next_submission_missing_location(self): data = {} - d = self.peer_module.get_next_submission(data) - self.assertFalse(d['success']) - self.assertEqual(d['error'], "Missing required keys: location") + response = self.peer_module.get_next_submission(data) + self.assertFalse(response['success']) + self.assertEqual(response['error'], 'Missing required keys: location') def test_save_grade_success(self): data = { @@ -345,9 +344,9 @@ def fake_get_item(key): def test_save_grade_missing_keys(self): data = {} - d = self.peer_module.save_grade(data) - self.assertFalse(d['success']) - self.assertTrue(d['error'].find('Missing required keys:') > -1) + response = self.peer_module.save_grade(data) + self.assertFalse(response['success']) + self.assertTrue(response['error'].find('Missing required keys:') > -1) def test_is_calibrated_success(self): data = {'location': self.location_string} diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py index 273178e9f0fe..7728ab8d7f29 100644 --- a/lms/djangoapps/open_ended_grading/utils.py +++ b/lms/djangoapps/open_ended_grading/utils.py @@ -1,7 +1,7 @@ import logging from xmodule.modulestore import search -from xmodule.modulestore.django import modulestore, ModuleI18nService +from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 1fe0d82e3921..dd8189bd6e5a 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -10,7 +10,7 @@ import json from student.models import unique_id_for_user -import open_ended_notifications +from open_ended_grading import open_ended_notifications from xmodule.modulestore.django import modulestore from xmodule.modulestore import search @@ -316,10 +316,10 @@ def take_action_on_flags(request, course_id): } return HttpResponse(json.dumps(response), mimetype="application/json") - p = request.POST - submission_id = p['submission_id'] - action_type = p['action_type'] - student_id = p['student_id'] + post = request.POST + submission_id = post['submission_id'] + action_type = post['action_type'] + student_id = post['student_id'] student_id = student_id.strip(' \t\n\r') submission_id = submission_id.strip(' \t\n\r') action_type = action_type.lower().strip(' \t\n\r') diff --git a/lms/djangoapps/psychometrics/management/commands/init_psychometrics.py b/lms/djangoapps/psychometrics/management/commands/init_psychometrics.py index 6ef183bcb13f..442039c06a5f 100644 --- a/lms/djangoapps/psychometrics/management/commands/init_psychometrics.py +++ b/lms/djangoapps/psychometrics/management/commands/init_psychometrics.py @@ -37,7 +37,7 @@ def handle(self, *args, **options): try: state = json.loads(sm.state) done = state['done'] - except: + except Exception: print "Oops, failed to eval state for %s (state=%s)" % (sm, sm.state) continue diff --git a/lms/djangoapps/psychometrics/models.py b/lms/djangoapps/psychometrics/models.py index 4af5544c6cc1..dbb73f0c182e 100644 --- a/lms/djangoapps/psychometrics/models.py +++ b/lms/djangoapps/psychometrics/models.py @@ -34,10 +34,12 @@ class PsychometricData(models.Model): # location = studentmodule.module_state_key def __unicode__(self): - sm = self.studentmodule - return "[PsychometricData] %s url=%s, grade=%s, max=%s, attempts=%s, ct=%s" % (sm.student, - sm.module_state_key, - sm.grade, - sm.max_grade, - self.attempts, - self.checktimes) + student_module = self.studentmodule + return "[PsychometricData] %s url=%s, grade=%s, max=%s, attempts=%s, ct=%s" % ( + student_module.student, + student_module.module_state_key, + student_module.grade, + student_module.max_grade, + self.attempts, + self.checktimes, + ) diff --git a/lms/djangoapps/psychometrics/psychoanalyze.py b/lms/djangoapps/psychometrics/psychoanalyze.py index bc164219c467..e0132b9b611d 100644 --- a/lms/djangoapps/psychometrics/psychoanalyze.py +++ b/lms/djangoapps/psychometrics/psychoanalyze.py @@ -30,12 +30,12 @@ # fit functions -def func_2pl(x, a, b): +def func_2pl(x_axis, a, b): """ 2-parameter logistic function """ D = 1.7 - edax = np.exp(D * a * (x - b)) + edax = np.exp(D * a * (x_axis - b)) return edax / (1 + edax) #----------------------------------------------------------------------------- @@ -54,21 +54,21 @@ def __init__(self, unit=1): self.min = None self.max = None - def add(self, x): - if x is None: + def add(self, x_axis): + if x_axis is None: return if self.min is None: - self.min = x + self.min = x_axis else: - if x < self.min: - self.min = x + if x_axis < self.min: + self.min = x_axis if self.max is None: - self.max = x + self.max = x_axis else: - if x > self.max: - self.max = x - self.sum += x - self.sum2 += x ** 2 + if x_axis > self.max: + self.max = x_axis + self.sum += x_axis + self.sum2 += x_axis ** 2 self.cnt += 1 def avg(self): @@ -82,17 +82,17 @@ def var(self): return (self.sum2 / 1.0 / self.cnt / (self.unit ** 2)) - (self.avg() ** 2) def sdv(self): - v = self.var() - if v > 0: - return math.sqrt(v) + value = self.var() + if value > 0: + return math.sqrt(value) else: return 0 def __str__(self): return 'cnt=%d, avg=%f, sdv=%f' % (self.cnt, self.avg(), self.sdv()) - def __add__(self, x): - self.add(x) + def __add__(self, x_axis): + self.add(x_axis) return self #----------------------------------------------------------------------------- @@ -112,9 +112,9 @@ def make_histogram(ydata, bins=None): nbins = len(bins) hist = dict(zip(bins, [0] * nbins)) - for y in ydata: + for y_axis in ydata: for b in bins[::-1]: # in reverse order - if y > b: + if y_axis > b: hist[b] += 1 break # hist['bins'] = bins @@ -171,8 +171,8 @@ def generate_plots_for_problem(problem): # compute grade statistics grades = [pmd.studentmodule.grade for pmd in pmdset] gsv = StatVar() - for g in grades: - gsv += g + for grade in grades: + gsv += grade msg += "

Grade distribution: %s

" % gsv # generate grade histogram @@ -196,12 +196,13 @@ def generate_plots_for_problem(problem): ghist = make_histogram(grades, np.linspace(0, max_grade, max_grade + 1)) ghist_json = json.dumps(ghist.items()) - plot = {'title': "Grade histogram for %s" % problem, - 'id': 'histogram', - 'info': '', - 'data': "var dhist = %s;\n" % ghist_json, - 'cmd': '[ {data: dhist, bars: { show: true, align: "center" }} ], %s' % axisopts, - } + plot = { + 'title': 'Grade histogram for %s' % problem, + 'id': 'histogram', + 'info': '', + 'data': 'var dhist = %s;\n' % ghist_json, + 'cmd': '[ {data: dhist, bars: { show: true, align: "center" }} ], %s' % axisopts, + } plots.append(plot) else: msg += "
Not generating histogram: max_grade=%s" % max_grade @@ -213,32 +214,33 @@ def generate_plots_for_problem(problem): for pmd in pmdset: try: checktimes = eval(pmd.checktimes) # update log of attempt timestamps - except: + except Exception: continue if len(checktimes) < 2: continue ct0 = checktimes[0] - for ct in checktimes[1:]: - dt = (ct - ct0).total_seconds() / 60.0 - if dt < 20: # ignore if dt too long - dtset.append(dt) - dtsv += dt - ct0 = ct + for check_time in checktimes[1:]: + delta = (check_time - ct0).total_seconds() / 60.0 + if delta < 20: # ignore if delta too long + dtset.append(delta) + dtsv += delta + ct0 = check_time if dtsv.cnt > 2: msg += "

Time differences between checks: %s

" % dtsv bins = np.linspace(0, 1.5 * dtsv.sdv(), 30) dbar = bins[1] - bins[0] thist = make_histogram(dtset, bins) - thist_json = json.dumps(sorted(thist.items(), key=lambda(x): x[0])) + thist_json = json.dumps(sorted(thist.items(), key=lambda(x_axis): x_axis[0])) axisopts = """{ xaxes: [{ axisLabel: 'Time (min)'}], yaxes: [{position: 'left',axisLabel: 'Count'}]}""" - plot = {'title': "Histogram of time differences between checks", - 'id': 'thistogram', - 'info': '', - 'data': "var thist = %s;\n" % thist_json, - 'cmd': '[ {data: thist, bars: { show: true, align: "center", barWidth:%f }} ], %s' % (dbar, axisopts), - } + plot = { + 'title': 'Histogram of time differences between checks', + 'id': 'thistogram', + 'info': '', + 'data': 'var thist = %s;\n' % thist_json, + 'cmd': '[ {data: thist, bars: { show: true, align: "center", barWidth:%f }} ], %s' % (dbar, axisopts), + } plots.append(plot) # one IRT plot curve for each grade received (TODO: this assumes integer grades) @@ -250,10 +252,10 @@ def generate_plots_for_problem(problem): continue ydat = [] ylast = 0 - for x in xdat: - y = gset.filter(attempts=x).count() / ngset - ydat.append(y + ylast) - ylast = y + ylast + for x_axis in xdat: + y_axis = gset.filter(attempts=x_axis).count() / ngset + ydat.append(y_axis + ylast) + ylast = y_axis + ylast yset['ydat'] = ydat if len(ydat) > 3: # try to fit to logistic function if enough data points @@ -266,7 +268,10 @@ def generate_plots_for_problem(problem): yset['fitx'] = fitx yset['fity'] = func_2pl(np.array(fitx), *cfp[0]) except Exception as err: - log.debug('Error in psychoanalyze curve fitting: %s' % err) + log.debug( + "Error in psychoanalyze curve fitting: %s", + err, + ) dataset['grade_%d' % grade] = yset @@ -298,12 +303,13 @@ def generate_plots_for_problem(problem): else: irtinfo = "" - plots.append({'title': 'IRT Plot for grade=%s %s' % (grade, irtinfo), - 'id': "irt%s" % grade, - 'info': '', - 'data': jsdata, - 'cmd': '[%s], %s' % (','.join(jsplots), axisopts), - }) + plots.append({ + 'title': 'IRT Plot for grade=%s %s' % (grade, irtinfo), + 'id': 'irt%s' % grade, + 'info': '', + 'data': jsdata, + 'cmd': '[%s], %s' % (','.join(jsplots), axisopts), + }) #log.debug('plots = %s' % plots) return msg, plots @@ -316,7 +322,7 @@ def make_psychometrics_data_update_handler(course_id, user, module_state_key): Construct and return a procedure which may be called to update the PsychometricData instance for the given StudentModule instance. """ - sm, status = StudentModule.objects.get_or_create( + student_module, status = StudentModule.objects.get_or_create( course_id=course_id, student=user, module_state_key=module_state_key, @@ -324,9 +330,9 @@ def make_psychometrics_data_update_handler(course_id, user, module_state_key): ) try: - pmd = PsychometricData.objects.using(db).get(studentmodule=sm) + pmd = PsychometricData.objects.using(db).get(studentmodule=student_module) except PsychometricData.DoesNotExist: - pmd = PsychometricData(studentmodule=sm) + pmd = PsychometricData(studentmodule=student_module) def psychometrics_data_update_handler(state): """ @@ -336,27 +342,38 @@ def psychometrics_data_update_handler(state): state = instance state (a nice, uniform way to interface - for more future psychometric feature extraction) """ try: - state = json.loads(sm.state) + state = json.loads(student_module.state) done = state['done'] - except: - log.exception("Oops, failed to eval state for %s (state=%s)" % (sm, sm.state)) + except Exception: + log.exception( + "Oops, failed to eval state for %s (state=%s)", + student_module, + student_module.state, + ) return pmd.done = done try: pmd.attempts = state.get('attempts', 0) - except: - log.exception("no attempts for %s (state=%s)" % (sm, sm.state)) + except Exception: + log.exception( + "no attempts for %s (state=%s)", + student_module, + student_module.state, + ) try: checktimes = eval(pmd.checktimes) # update log of attempt timestamps - except: + except Exception: checktimes = [] checktimes.append(datetime.datetime.now(UTC)) pmd.checktimes = checktimes try: pmd.save() - except: - log.exception("Error in updating psychometrics data for %s" % sm) + except Exception: + log.exception( + "Error in updating psychometrics data for %s", + student_module, + ) return psychometrics_data_update_handler diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index cd50e8f97017..8b0c1c726809 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -1930,7 +1930,7 @@ def _tax_deduction_msg(self): ).format(platform_name=settings.PLATFORM_NAME) @classmethod - def _line_item_description(self, course_id=None): + def _line_item_description(cls, course_id=None): """Create a line-item description for the donation. Includes the course display name if provided. diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource.py b/lms/djangoapps/shoppingcart/processors/CyberSource.py index 0e6c12939b6d..9d1334d18767 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource.py +++ b/lms/djangoapps/shoppingcart/processors/CyberSource.py @@ -219,9 +219,9 @@ def record_purchase(params, order): Record the purchase and run purchased_callbacks """ ccnum_str = params.get('card_accountNumber', '') - m = re.search("\d", ccnum_str) - if m: - ccnum = ccnum_str[m.start():] + match = re.search('\d', ccnum_str) + if match: + ccnum = ccnum_str[match.start():] else: ccnum = "####" diff --git a/lms/djangoapps/shoppingcart/processors/CyberSource2.py b/lms/djangoapps/shoppingcart/processors/CyberSource2.py index 37450b084e2f..9c9e09705f94 100644 --- a/lms/djangoapps/shoppingcart/processors/CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/CyberSource2.py @@ -401,9 +401,9 @@ def _record_purchase(params, order): # Parse the string to retrieve the digits. # If we can't find any digits, use placeholder values instead. ccnum_str = params.get('req_card_number', '') - mm = re.search("\d", ccnum_str) - if mm: - ccnum = ccnum_str[mm.start():] + match = re.search('\d', ccnum_str) + if match: + ccnum = ccnum_str[match.start():] else: ccnum = "####" diff --git a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py index d719e06f8988..c452e1a4940d 100644 --- a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py +++ b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource.py @@ -143,14 +143,14 @@ def test_get_processor_decline_html(self): """ Tests the processor decline html message """ - DECISION = 'REJECT' + decision = 'REJECT' for code, reason in REASONCODE_MAP.iteritems(): params = { - 'decision': DECISION, + 'decision': decision, 'reasonCode': code, } html = get_processor_decline_html(params) - self.assertIn(DECISION, html) + self.assertIn(decision, html) self.assertIn(reason, html) self.assertIn(code, html) self.assertIn(settings.PAYMENT_SUPPORT_EMAIL, html) @@ -159,10 +159,10 @@ def test_get_processor_exception_html(self): """ Tests the processor exception html message """ - for type in [CCProcessorSignatureException, CCProcessorWrongAmountException, CCProcessorDataException]: - error_msg = "An exception message of with exception type {0}".format(str(type)) - exception = type(error_msg) - html = get_processor_exception_html(exception) + for exception_type in [CCProcessorSignatureException, CCProcessorWrongAmountException, CCProcessorDataException]: + error_msg = "An exception message of with exception type {0}".format(str(exception_type)) + exception = exception_type(error_msg) + html = get_processor_exception_html(exception_type) self.assertIn(settings.PAYMENT_SUPPORT_EMAIL, html) self.assertIn('Sorry!', html) self.assertIn(error_msg, html) diff --git a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py index 62b7a1924af3..4f8551af86c2 100644 --- a/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py +++ b/lms/djangoapps/shoppingcart/processors/tests/test_CyberSource2.py @@ -283,9 +283,14 @@ def test_get_processor_exception_html(self, error_string): self.assertIn(error_msg, html) def _signed_callback_params( - self, order_id, order_amount, paid_amount, - decision='ACCEPT', signature=None, card_number='xxxxxxxxxxxx1111', - first_name='John' + self, + order_id, + order_amount, + paid_amount, + decision='ACCEPT', + signature=None, + card_number='xxxxxxxxxxxx1111', + first_name='John', ): """ Construct parameters that could be returned from CyberSource diff --git a/lms/djangoapps/shoppingcart/tests/test_context_processor.py b/lms/djangoapps/shoppingcart/tests/test_context_processor.py index fe232a66da9f..010504c6edfc 100644 --- a/lms/djangoapps/shoppingcart/tests/test_context_processor.py +++ b/lms/djangoapps/shoppingcart/tests/test_context_processor.py @@ -3,11 +3,9 @@ """ from django.conf import settings from django.contrib.auth.models import AnonymousUser -from django.test.utils import override_settings from mock import patch, Mock from course_modes.tests.factories import CourseModeFactory -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory diff --git a/lms/djangoapps/shoppingcart/tests/test_microsites.py b/lms/djangoapps/shoppingcart/tests/test_microsites.py index c7e430e1b3be..9fe0b5fcda16 100644 --- a/lms/djangoapps/shoppingcart/tests/test_microsites.py +++ b/lms/djangoapps/shoppingcart/tests/test_microsites.py @@ -4,13 +4,12 @@ import mock from django.conf import settings -from django.test.utils import override_settings from django.core.urlresolvers import reverse from mock import patch from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, mixed_store_config + ModuleStoreTestCase, ) from xmodule.modulestore.tests.factories import CourseFactory from shoppingcart.models import ( diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index bc2f9ae2f00b..a39825ecf9b3 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -455,8 +455,15 @@ def test_add_to_order(self): self.assertEqual(reg1.user, self.user) self.assertEqual(reg1.status, "cart") self.assertTrue(PaidCourseRegistration.contained_in_order(self.cart, self.course_key)) - self.assertFalse(PaidCourseRegistration.contained_in_order( - self.cart, CourseLocator(org="MITx", course="999", run="Robot_Super_Course_abcd")) + self.assertFalse( + PaidCourseRegistration.contained_in_order( + self.cart, + CourseLocator( + org='MITx', + course='999', + run='Robot_Super_Course_abcd', + ), + ), ) self.assertEqual(self.cart.total_cost, self.cost) diff --git a/lms/djangoapps/shoppingcart/tests/test_reports.py b/lms/djangoapps/shoppingcart/tests/test_reports.py index 1083d4fbfc4f..388f701cb391 100644 --- a/lms/djangoapps/shoppingcart/tests/test_reports.py +++ b/lms/djangoapps/shoppingcart/tests/test_reports.py @@ -9,10 +9,8 @@ from textwrap import dedent from django.conf import settings -from django.test.utils import override_settings from course_modes.models import CourseMode -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from shoppingcart.models import (Order, CertificateItem, PaidCourseRegistration, PaidCourseRegistrationAnnotation, CourseRegCodeItemAnnotation) from shoppingcart.views import initialize_report diff --git a/lms/djangoapps/shoppingcart/tests/test_views.py b/lms/djangoapps/shoppingcart/tests/test_views.py index fb5606095cfb..5da345328a1d 100644 --- a/lms/djangoapps/shoppingcart/tests/test_views.py +++ b/lms/djangoapps/shoppingcart/tests/test_views.py @@ -25,7 +25,7 @@ import ddt from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, mixed_store_config + ModuleStoreTestCase, ) from xmodule.modulestore.tests.factories import CourseFactory from student.roles import CourseSalesAdminRole @@ -51,15 +51,15 @@ def mock_render_purchase_form_html(*args, **kwargs): return render_purchase_form_html(*args, **kwargs) -form_mock = Mock(side_effect=mock_render_purchase_form_html) +MOCK_FORM = Mock(side_effect=mock_render_purchase_form_html) def mock_render_to_response(*args, **kwargs): return render_to_response(*args, **kwargs) -render_mock = Mock(side_effect=mock_render_to_response) +MOCK_RENDER = Mock(side_effect=mock_render_to_response) -postpay_mock = Mock() +MOCK_POST_PAY = Mock() @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True}) @@ -155,7 +155,7 @@ def test_add_course_to_cart_anon(self): resp = self.client.post(reverse('shoppingcart.views.add_course_to_cart', args=[self.course_key.to_deprecated_string()])) self.assertEqual(resp.status_code, 403) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_billing_details(self): billing_url = reverse('billing_details') self.login_user() @@ -170,7 +170,7 @@ def test_billing_details(self): resp = self.client.get(billing_url) self.assertEqual(resp.status_code, 200) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/billing_details.html') # check for the default currency in the context self.assertEqual(context['currency'], 'usd') @@ -186,7 +186,7 @@ def test_billing_details(self): resp = self.client.post(billing_url, data) self.assertEqual(resp.status_code, 200) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=['PKR', 'Rs']) def test_billing_details_with_override_currency_settings(self): billing_url = reverse('billing_details') @@ -198,7 +198,7 @@ def test_billing_details_with_override_currency_settings(self): resp = self.client.get(billing_url) self.assertEqual(resp.status_code, 200) - ((template, context), __) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), __) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/billing_details.html') # check for the override currency settings in the context @@ -719,8 +719,8 @@ def test_add_course_to_cart_success(self): self.assertEqual(resp.status_code, 200) self.assertTrue(PaidCourseRegistration.contained_in_order(self.cart, self.course_key)) - @patch('shoppingcart.views.render_purchase_form_html', form_mock) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_purchase_form_html', MOCK_FORM) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_cart(self): self.login_user() reg_item = PaidCourseRegistration.add_to_order(self.cart, self.course_key) @@ -728,13 +728,13 @@ def test_show_cart(self): resp = self.client.get(reverse('shoppingcart.views.show_cart', args=[])) self.assertEqual(resp.status_code, 200) - ((purchase_form_arg_cart,), _) = form_mock.call_args # pylint: disable=redefined-outer-name + ((purchase_form_arg_cart,), _) = MOCK_FORM.call_args # pylint: disable=redefined-outer-name purchase_form_arg_cart_items = purchase_form_arg_cart.orderitem_set.all().select_subclasses() self.assertIn(reg_item, purchase_form_arg_cart_items) self.assertIn(cert_item, purchase_form_arg_cart_items) self.assertEqual(len(purchase_form_arg_cart_items), 2) - ((template, context), _) = render_mock.call_args + ((template, context), _) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/shopping_cart.html') self.assertEqual(len(context['shoppingcart_items']), 2) self.assertEqual(context['amount'], 80) @@ -743,8 +743,8 @@ def test_show_cart(self): self.assertEqual(context['currency'], 'usd') self.assertEqual(context['currency_symbol'], '$') - @patch('shoppingcart.views.render_purchase_form_html', form_mock) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_purchase_form_html', MOCK_FORM) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=['PKR', 'Rs']) def test_show_cart_with_override_currency_settings(self): self.login_user() @@ -752,11 +752,11 @@ def test_show_cart_with_override_currency_settings(self): resp = self.client.get(reverse('shoppingcart.views.show_cart', args=[])) self.assertEqual(resp.status_code, 200) - ((purchase_form_arg_cart,), _) = form_mock.call_args # pylint: disable=redefined-outer-name + ((purchase_form_arg_cart,), _) = MOCK_FORM.call_args # pylint: disable=redefined-outer-name purchase_form_arg_cart_items = purchase_form_arg_cart.orderitem_set.all().select_subclasses() self.assertIn(reg_item, purchase_form_arg_cart_items) - ((template, context), _) = render_mock.call_args + ((template, context), _) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/shopping_cart.html') # check for the override currency settings in the context self.assertEqual(context['currency'], 'PKR') @@ -801,25 +801,32 @@ def test_remove_item(self, exception_log): '-1' ) - @patch('shoppingcart.views.process_postpay_callback', postpay_mock) + @patch('shoppingcart.views.process_postpay_callback', MOCK_POST_PAY) def test_postpay_callback_success(self): - postpay_mock.return_value = {'success': True, 'order': self.cart} + MOCK_POST_PAY.return_value = { + 'success': True, + 'order': self.cart, + } self.login_user() resp = self.client.post(reverse('shoppingcart.views.postpay_callback', args=[])) self.assertEqual(resp.status_code, 302) self.assertEqual(urlparse(resp.__getitem__('location')).path, reverse('shoppingcart.views.show_receipt', args=[self.cart.id])) - @patch('shoppingcart.views.process_postpay_callback', postpay_mock) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.process_postpay_callback', MOCK_POST_PAY) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_postpay_callback_failure(self): - postpay_mock.return_value = {'success': False, 'order': self.cart, 'error_html': 'ERROR_TEST!!!'} + MOCK_POST_PAY.return_value = { + 'success': False, + 'order': self.cart, + 'error_html': 'ERROR_TEST!!!', + } self.login_user() resp = self.client.post(reverse('shoppingcart.views.postpay_callback', args=[])) self.assertEqual(resp.status_code, 200) self.assertIn('ERROR_TEST!!!', resp.content) - ((template, context), _) = render_mock.call_args + ((template, context), _) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/error.html') self.assertEqual(context['order'], self.cart) self.assertEqual(context['error_html'], 'ERROR_TEST!!!') @@ -959,7 +966,7 @@ def test_total_amount_of_purchased_course(self): total_amount = PaidCourseRegistration.get_total_amount_of_purchased_item(self.course_key) self.assertEqual(total_amount, 76) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_with_valid_coupon_code(self): self.add_course_to_user_cart(self.course_key) self.add_coupon(self.course_key, True, self.coupon_code) @@ -973,7 +980,7 @@ def test_show_receipt_success_with_valid_coupon_code(self): self.assertIn('FirstNameTesting123', resp.content) self.assertIn(str(self.get_discount(self.cost)), resp.content) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_reg_code_and_course_registration_scenario(self): self.add_reg_code(self.course_key) @@ -994,7 +1001,7 @@ def test_reg_code_and_course_registration_scenario(self): response = self.client.post(redeem_url) self.assertEquals(response.status_code, 200) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_reg_code_with_multiple_courses_and_checkout_scenario(self): self.add_reg_code(self.course_key) @@ -1024,7 +1031,7 @@ def test_reg_code_with_multiple_courses_and_checkout_scenario(self): resp = self.client.get(reverse('shoppingcart.views.show_receipt', args=[self.cart.id])) self.assertEqual(resp.status_code, 200) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/receipt.html') self.assertEqual(context['order'], self.cart) self.assertEqual(context['order'].total_cost, self.testing_cost) @@ -1040,7 +1047,7 @@ def test_reg_code_with_multiple_courses_and_checkout_scenario(self): self.assertIsNotNone(item2.course_enrollment) self.assertEqual(item2.course_enrollment.course_id, self.testing_course.id) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_with_valid_reg_code(self): self.add_course_to_user_cart(self.course_key) self.add_reg_code(self.course_key) @@ -1053,7 +1060,7 @@ def test_show_receipt_success_with_valid_reg_code(self): self.assertEqual(resp.status_code, 200) self.assertIn('0.00', resp.content) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success(self): reg_item = PaidCourseRegistration.add_to_order(self.cart, self.course_key) cert_item = CertificateItem.add_to_order(self.cart, self.verified_course_key, self.cost, 'honor') @@ -1065,7 +1072,7 @@ def test_show_receipt_success(self): self.assertIn('FirstNameTesting123', resp.content) self.assertIn('80.00', resp.content) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/receipt.html') self.assertEqual(context['order'], self.cart) self.assertIn(reg_item, context['shoppingcart_items'][0]) @@ -1076,7 +1083,7 @@ def test_show_receipt_success(self): self.assertEqual(context['currency'], 'usd') @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=['PKR', 'Rs']) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_with_override_currency_settings(self): reg_item = PaidCourseRegistration.add_to_order(self.cart, self.course_key) cert_item = CertificateItem.add_to_order(self.cart, self.verified_course_key, self.cost, 'honor') @@ -1086,7 +1093,7 @@ def test_show_receipt_success_with_override_currency_settings(self): resp = self.client.get(reverse('shoppingcart.views.show_receipt', args=[self.cart.id])) self.assertEqual(resp.status_code, 200) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/receipt.html') self.assertIn(reg_item, context['shoppingcart_items'][0]) self.assertIn(cert_item, context['shoppingcart_items'][1]) @@ -1095,7 +1102,7 @@ def test_show_receipt_success_with_override_currency_settings(self): self.assertEqual(context['currency_symbol'], 'Rs') self.assertEqual(context['currency'], 'PKR') - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_courseregcode_item_total_price(self): self.cart.order_type = 'business' self.cart.save() @@ -1103,7 +1110,7 @@ def test_courseregcode_item_total_price(self): self.cart.purchase(first='FirstNameTesting123', street1='StreetTesting123') self.assertEquals(CourseRegCodeItem.get_total_amount_of_purchased_item(self.course_key), 80) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_with_order_type_business(self): self.cart.order_type = 'business' self.cart.save() @@ -1130,7 +1137,7 @@ def test_show_receipt_success_with_order_type_business(self): # fetch the newly generated registration codes course_registration_codes = CourseRegistrationCode.objects.filter(order=self.cart) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/receipt.html') self.assertEqual(context['order'], self.cart) self.assertIn(reg_item, context['shoppingcart_items'][0]) @@ -1162,14 +1169,14 @@ def test_show_receipt_success_with_order_type_business(self): # has been expired or not resp = self.client.get(reverse('shoppingcart.views.show_receipt', args=[self.cart.id])) self.assertEqual(resp.status_code, 200) - ((template, context), _) = render_mock.call_args # pylint: disable=redefined-outer-name + ((template, context), _) = MOCK_RENDER.call_args # pylint: disable=redefined-outer-name self.assertEqual(template, 'shoppingcart/receipt.html') # now check for all the registration codes in the receipt # and one of code should be used at this point self.assertTrue(context['reg_code_info_list'][0]['is_redeemed']) self.assertFalse(context['reg_code_info_list'][1]['is_redeemed']) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_with_upgrade(self): reg_item = PaidCourseRegistration.add_to_order(self.cart, self.course_key) @@ -1179,9 +1186,9 @@ def test_show_receipt_success_with_upgrade(self): self.login_user() # When we come from the upgrade flow, we'll have a session variable showing that - s = self.client.session - s['attempting_upgrade'] = True - s.save() + session = self.client.session + session['attempting_upgrade'] = True + session.save() self.mock_tracker.emit.reset_mock() # pylint: disable=maybe-no-member resp = self.client.get(reverse('shoppingcart.views.show_receipt', args=[self.cart.id])) @@ -1194,7 +1201,7 @@ def test_show_receipt_success_with_upgrade(self): self.assertIn('FirstNameTesting123', resp.content) self.assertIn('80.00', resp.content) - ((template, context), _) = render_mock.call_args + ((template, context), _) = MOCK_RENDER.call_args # When we come from the upgrade flow, we get these context variables @@ -1215,7 +1222,7 @@ def test_show_receipt_success_with_upgrade(self): } ) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_refund(self): reg_item = PaidCourseRegistration.add_to_order(self.cart, self.course_key) cert_item = CertificateItem.add_to_order(self.cart, self.verified_course_key, self.cost, 'honor') @@ -1228,14 +1235,14 @@ def test_show_receipt_success_refund(self): self.assertEqual(resp.status_code, 200) self.assertIn('40.00', resp.content) - ((template, context), _tmp) = render_mock.call_args + ((template, context), _tmp) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/receipt.html') self.assertEqual(context['order'], self.cart) self.assertIn(reg_item, context['shoppingcart_items'][0]) self.assertIn(cert_item, context['shoppingcart_items'][1]) self.assertTrue(context['any_refunds']) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_show_receipt_success_custom_receipt_page(self): cert_item = CertificateItem.add_to_order(self.cart, self.course_key, self.cost, 'honor') self.cart.purchase() @@ -1243,7 +1250,7 @@ def test_show_receipt_success_custom_receipt_page(self): receipt_url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id]) resp = self.client.get(receipt_url) self.assertEqual(resp.status_code, 200) - ((template, _context), _tmp) = render_mock.call_args + ((template, _context), _tmp) = MOCK_RENDER.call_args self.assertEqual(template, cert_item.single_item_receipt_template) def _assert_404(self, url, use_post=False): @@ -1378,7 +1385,7 @@ def login_user(self): """ self.client.login(username=self.user.username, password="password") - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_to_check_that_cart_item_enrollment_is_closed(self): self.login_user() reg_item1 = PaidCourseRegistration.add_to_order(self.cart, self.course_key) @@ -1395,7 +1402,7 @@ def test_to_check_that_cart_item_enrollment_is_closed(self): self.assertEqual(resp.status_code, 200) self.assertIn("{course_name} has been removed because the enrollment period has closed.".format(course_name=self.testing_course.display_name), resp.content) - ((template, context), _tmp) = render_mock.call_args + ((template, context), _tmp) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/shopping_cart.html') self.assertEqual(context['order'], self.cart) self.assertIn(reg_item1, context['shoppingcart_items'][0]) @@ -1788,25 +1795,25 @@ def test_report_csv_bad_method(self): response = self.client.put(reverse('payment_csv_report')) self.assertEqual(response.status_code, 400) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_report_csv_get(self): self.login_user() self.add_to_download_group(self.user) response = self.client.get(reverse('payment_csv_report')) - ((template, context), unused_kwargs) = render_mock.call_args + ((template, context), unused_kwargs) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/download_report.html') self.assertFalse(context['total_count_error']) self.assertFalse(context['date_fmt_error']) self.assertIn(_("Download CSV Reports"), response.content.decode('UTF-8')) - @patch('shoppingcart.views.render_to_response', render_mock) + @patch('shoppingcart.views.render_to_response', MOCK_RENDER) def test_report_csv_bad_date(self): self.login_user() self.add_to_download_group(self.user) response = self.client.post(reverse('payment_csv_report'), {'start_date': 'BAD', 'end_date': 'BAD', 'requested_report': 'itemized_purchase_report'}) - ((template, context), unused_kwargs) = render_mock.call_args + ((template, context), unused_kwargs) = MOCK_RENDER.call_args self.assertEqual(template, 'shoppingcart/download_report.html') self.assertFalse(context['total_count_error']) self.assertTrue(context['date_fmt_error']) diff --git a/lms/djangoapps/shoppingcart/urls.py b/lms/djangoapps/shoppingcart/urls.py index 79e83bc7e531..83754f10aa61 100644 --- a/lms/djangoapps/shoppingcart/urls.py +++ b/lms/djangoapps/shoppingcart/urls.py @@ -1,27 +1,76 @@ from django.conf.urls import patterns, url from django.conf import settings -urlpatterns = patterns('shoppingcart.views', # nopep8 - url(r'^postpay_callback/$', 'postpay_callback'), # Both the ~accept and ~reject callback pages are handled here - url(r'^receipt/(?P[0-9]*)/$', 'show_receipt'), - url(r'^donation/$', 'donate', name='donation'), - url(r'^csv_report/$', 'csv_report', name='payment_csv_report'), +urlpatterns = patterns( + 'shoppingcart.views', + url( + r'^postpay_callback/$', + 'postpay_callback', + ), # Both the ~accept and ~reject callback pages are handled here + url( + r'^receipt/(?P[0-9]*)/$', + 'show_receipt', + ), + url( + r'^donation/$', + 'donate', + name='donation', + ), + url( + r'^csv_report/$', + 'csv_report', + name='payment_csv_report', + ), # These following URLs are only valid if the ENABLE_SHOPPING_CART feature flag is set - url(r'^$', 'show_cart'), - url(r'^clear/$', 'clear_cart'), - url(r'^remove_item/$', 'remove_item'), - url(r'^add/course/{}/$'.format(settings.COURSE_ID_PATTERN), 'add_course_to_cart', name='add_course_to_cart'), - url(r'^register/redeem/(?P[0-9A-Za-z]+)/$', 'register_code_redemption', name='register_code_redemption'), - url(r'^use_code/$', 'use_code'), - url(r'^update_user_cart/$', 'update_user_cart'), - url(r'^reset_code_redemption/$', 'reset_code_redemption'), + url( + r'^$', + 'show_cart', + ), + url( + r'^clear/$', + 'clear_cart', + ), + url( + r'^remove_item/$', + 'remove_item', + ), + url( + r'^add/course/{}/$'.format( + settings.COURSE_ID_PATTERN, + ), + 'add_course_to_cart', + name='add_course_to_cart', + ), + url( + r'^register/redeem/(?P[0-9A-Za-z]+)/$', + 'register_code_redemption', + name='register_code_redemption', + ), + url( + r'^use_code/$', + 'use_code', + ), + url( + r'^update_user_cart/$', + 'update_user_cart', + ), + url( + r'^reset_code_redemption/$', + 'reset_code_redemption', + ), url(r'^billing_details/$', 'billing_details', name='billing_details'), - url(r'^verify_cart/$', 'verify_cart'), + url( + r'^verify_cart/$', + 'verify_cart', + ), ) if settings.FEATURES.get('ENABLE_PAYMENT_FAKE'): from shoppingcart.tests.payment_fake import PaymentFakeView urlpatterns += patterns( 'shoppingcart.tests.payment_fake', - url(r'^payment_fake', PaymentFakeView.as_view()), + url( + r'^payment_fake', + PaymentFakeView.as_view(), + ), ) diff --git a/lms/djangoapps/staticbook/tests.py b/lms/djangoapps/staticbook/tests.py index 766fb62d09ef..8605e94ad706 100644 --- a/lms/djangoapps/staticbook/tests.py +++ b/lms/djangoapps/staticbook/tests.py @@ -7,10 +7,8 @@ import mock import requests -from django.test.utils import override_settings from django.core.urlresolvers import reverse, NoReverseMatch -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from student.tests.factories import UserFactory, CourseEnrollmentFactory from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index af2e2080e385..a89db6449df3 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -20,7 +20,7 @@ from openedx.core.djangoapps.user_api.api import account as account_api from openedx.core.djangoapps.user_api.api import profile as profile_api from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, mixed_store_config + ModuleStoreTestCase, ) from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import CourseModeFactory diff --git a/lms/djangoapps/survey/admin.py b/lms/djangoapps/survey/admin.py index 26b0cfd71b96..9552789b9845 100644 --- a/lms/djangoapps/survey/admin.py +++ b/lms/djangoapps/survey/admin.py @@ -7,10 +7,10 @@ from survey.models import SurveyForm -class SurveyFormAdminForm(forms.ModelForm): # pylint: disable=incomplete-protocol +class SurveyFormAdminForm(forms.ModelForm): """Form providing validation of SurveyForm content.""" - class Meta: # pylint: disable=missing-docstring + class Meta(object): # pylint: disable=missing-docstring model = SurveyForm fields = ('name', 'form') diff --git a/lms/djangoapps/survey/tests/test_utils.py b/lms/djangoapps/survey/tests/test_utils.py index 2128d3d1d6e8..320a4ad93cc5 100644 --- a/lms/djangoapps/survey/tests/test_utils.py +++ b/lms/djangoapps/survey/tests/test_utils.py @@ -4,7 +4,6 @@ from collections import OrderedDict -from django.test import TestCase from django.test.client import Client from django.contrib.auth.models import User diff --git a/lms/djangoapps/survey/tests/test_views.py b/lms/djangoapps/survey/tests/test_views.py index 855829242bc6..2d1d718e87b5 100644 --- a/lms/djangoapps/survey/tests/test_views.py +++ b/lms/djangoapps/survey/tests/test_views.py @@ -5,7 +5,6 @@ import json from collections import OrderedDict -from django.test import TestCase from django.test.client import Client from django.contrib.auth.models import User from django.core.urlresolvers import reverse diff --git a/lms/djangoapps/survey/urls.py b/lms/djangoapps/survey/urls.py index 727602e9d4a5..e038e33fe866 100644 --- a/lms/djangoapps/survey/urls.py +++ b/lms/djangoapps/survey/urls.py @@ -5,7 +5,16 @@ from django.conf.urls import patterns, url -urlpatterns = patterns('survey.views', # nopep8 - url(r'^(?P[0-9A-Za-z]+)/$', 'view_survey', name='view_survey'), - url(r'^(?P[0-9A-Za-z]+)/answers/$', 'submit_answers', name='submit_answers'), +urlpatterns = patterns( + 'survey.views', + url( + r'^(?P[0-9A-Za-z]+)/$', + 'view_survey', + name='view_survey', + ), + url( + r'^(?P[0-9A-Za-z]+)/answers/$', + 'submit_answers', + name='submit_answers', + ), ) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 10bd7ad9ae42..3fa52c82efb6 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -140,7 +140,7 @@ class PhotoVerification(StatusModel): # user IDs or something too easily guessable. receipt_id = models.CharField( db_index=True, - default=lambda: generateUUID(), + default=generateUUID(), max_length=255, ) @@ -177,7 +177,7 @@ class PhotoVerification(StatusModel): # capturing it so that we can later query for the common problems. error_code = models.CharField(blank=True, max_length=50) - class Meta: + class Meta(object): abstract = True ordering = ['-created_at'] diff --git a/lms/djangoapps/verify_student/tests/test_integration.py b/lms/djangoapps/verify_student/tests/test_integration.py index de7995ddc082..9a50df7c78ed 100644 --- a/lms/djangoapps/verify_student/tests/test_integration.py +++ b/lms/djangoapps/verify_student/tests/test_integration.py @@ -2,13 +2,10 @@ Integration tests of the payment flow, including course mode selection. """ -from lxml.html import soupparser -from django.test.utils import override_settings from django.core.urlresolvers import reverse -from django.conf import settings from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from student.tests.factories import UserFactory from student.models import CourseEnrollment from course_modes.tests.factories import CourseModeFactory diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index 44ab2ebc0f03..fe5fddcebb66 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -6,12 +6,10 @@ from django.conf import settings from django.test import TestCase -from django.test.utils import override_settings from mock import patch from nose.tools import assert_is_none, assert_equals, assert_raises, assert_true, assert_false # pylint: disable=E0611 from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from reverification.tests.factories import MidcourseReverificationWindowFactory from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -311,7 +309,7 @@ def test_user_is_verified(self): attempt.status = "approved" attempt.save() - assert_true(SoftwareSecurePhotoVerification.user_is_verified(user), status) + assert_true(SoftwareSecurePhotoVerification.user_is_verified(user), attempt.status) def test_user_has_valid_or_pending(self): """ diff --git a/lms/djangoapps/verify_student/tests/test_ssencrypt.py b/lms/djangoapps/verify_student/tests/test_ssencrypt.py index 90cb5b3bec10..c8765a5f1b05 100644 --- a/lms/djangoapps/verify_student/tests/test_ssencrypt.py +++ b/lms/djangoapps/verify_student/tests/test_ssencrypt.py @@ -3,7 +3,8 @@ from verify_student.ssencrypt import ( aes_decrypt, aes_encrypt, encrypt_and_encode, decode_and_decrypt, - rsa_decrypt, rsa_encrypt, random_aes_key + rsa_decrypt, + rsa_encrypt, ) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 014e9db6527b..a96edf47780a 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -13,7 +13,6 @@ import ddt from django.test.client import Client from django.test import TestCase -from django.test.utils import override_settings from django.conf import settings from django.core.urlresolvers import reverse from django.core.exceptions import ObjectDoesNotExist @@ -21,7 +20,7 @@ from bs4 import BeautifulSoup from openedx.core.djangoapps.user_api.api import profile as profile_api -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum @@ -40,7 +39,7 @@ def mock_render_to_response(*args, **kwargs): return render_to_response(*args, **kwargs) -render_mock = Mock(side_effect=mock_render_to_response) +MOCK_RENDER = Mock(side_effect=mock_render_to_response) class StartView(TestCase): @@ -1367,21 +1366,21 @@ def setUp(self): self.course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') self.course_key = self.course.id - @patch('verify_student.views.render_to_response', render_mock) + @patch('verify_student.views.render_to_response', MOCK_RENDER) def test_reverify_get(self): url = reverse('verify_student_reverify') response = self.client.get(url) self.assertEquals(response.status_code, 200) - ((_template, context), _kwargs) = render_mock.call_args # pylint: disable=unpacking-non-sequence + ((_template, context), _kwargs) = MOCK_RENDER.call_args # pylint: disable=unpacking-non-sequence self.assertFalse(context['error']) - @patch('verify_student.views.render_to_response', render_mock) + @patch('verify_student.views.render_to_response', MOCK_RENDER) def test_reverify_post_failure(self): url = reverse('verify_student_reverify') response = self.client.post(url, {'face_image': '', 'photo_id_image': ''}) self.assertEquals(response.status_code, 200) - ((template, context), _kwargs) = render_mock.call_args # pylint: disable=unpacking-non-sequence + ((template, context), _kwargs) = MOCK_RENDER.call_args # pylint: disable=unpacking-non-sequence self.assertIn('photo_reverification', template) self.assertTrue(context['error']) @@ -1396,7 +1395,7 @@ def test_reverify_post_success(self): self.assertIsNotNone(verification_attempt) except ObjectDoesNotExist: self.fail('No verification object generated') - ((template, context), _kwargs) = render_mock.call_args # pylint: disable=unpacking-non-sequence + ((template, context), _kwargs) = MOCK_RENDER.call_args # pylint: disable=unpacking-non-sequence self.assertIn('photo_reverification', template) self.assertTrue(context['error']) @@ -1417,7 +1416,7 @@ def setUp(self): self.mock_tracker = patcher.start() self.addCleanup(patcher.stop) - @patch('verify_student.views.render_to_response', render_mock) + @patch('verify_student.views.render_to_response', MOCK_RENDER) def test_midcourse_reverify_get(self): url = reverse('verify_student_midcourse_reverify', kwargs={"course_id": self.course_key.to_deprecated_string()}) @@ -1447,7 +1446,7 @@ def test_midcourse_reverify_get(self): self.mock_tracker.emit.reset_mock() # pylint: disable=no-member self.assertEquals(response.status_code, 200) - ((_template, context), _kwargs) = render_mock.call_args # pylint: disable=unpacking-non-sequence + ((_template, context), _kwargs) = MOCK_RENDER.call_args # pylint: disable=unpacking-non-sequence self.assertFalse(context['error']) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) @@ -1500,7 +1499,7 @@ def test_midcourse_reverify_post_failure_expired_window(self): with self.assertRaises(ObjectDoesNotExist): SoftwareSecurePhotoVerification.objects.get(user=self.user, window=window) - @patch('verify_student.views.render_to_response', render_mock) + @patch('verify_student.views.render_to_response', MOCK_RENDER) def test_midcourse_reverify_dash(self): url = reverse('verify_student_midcourse_reverify_dash') response = self.client.get(url) @@ -1514,7 +1513,7 @@ def test_midcourse_reverify_dash(self): # enrolled in a verified course, and the window is open self.assertEquals(response.status_code, 200) - @patch('verify_student.views.render_to_response', render_mock) + @patch('verify_student.views.render_to_response', MOCK_RENDER) def test_midcourse_reverify_invalid_course_id(self): # if course id is invalid return 400 invalid_course_key = CourseLocator('edx', 'not', 'valid') diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 770efc871f28..b121f9a8e20a 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -219,10 +219,12 @@ class PayAndVerifyView(View): @method_decorator(login_required) def get( - self, request, course_id, - always_show_payment=False, - current_step=None, - message=FIRST_TIME_VERIFY_MSG + self, + request, + course_id, + always_show_payment=False, + current_step=None, + message=FIRST_TIME_VERIFY_MSG, ): """Render the pay/verify requirements page. @@ -374,12 +376,12 @@ def get( return render_to_response("verify_student/pay_and_verify.html", context) def _redirect_if_necessary( - self, - message, - already_verified, - already_paid, - is_enrolled, - course_key + self, + message, + already_verified, + already_paid, + is_enrolled, + course_key, ): """Redirect the user to a more appropriate page if necessary. @@ -598,8 +600,9 @@ def create_order(request): ) if ( - submit_photo and not - SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user) + submit_photo + and not + SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user) ): attempt = SoftwareSecurePhotoVerification(user=request.user) try: diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index c87aee700a7e..993dc9c8ef35 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -24,7 +24,6 @@ import os from random import choice -import string def seed(): diff --git a/lms/envs/common.py b/lms/envs/common.py index 3be88b59327b..017de2f4d93d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -90,8 +90,9 @@ # university to use for branding purposes 'SUBDOMAIN_BRANDING': False, - 'FORCE_UNIVERSITY_DOMAIN': False, # set this to the university domain to use, as an override to HTTP_HOST - # set to None to do no university selection + # Set this to the university domain to use, as an override to HTTP_HOST + # Set to None to do no university selection + 'FORCE_UNIVERSITY_DOMAIN': False, # for consistency in user-experience, keep the value of the following 3 settings # in sync with the corresponding ones in cms/envs/common.py diff --git a/lms/envs/dev_with_worker.py b/lms/envs/dev_with_worker.py index 40f6ed4e1ff1..fe151c291ad7 100644 --- a/lms/envs/dev_with_worker.py +++ b/lms/envs/dev_with_worker.py @@ -12,7 +12,7 @@ # want to import all variables from base settings files # pylint: disable=wildcard-import, unused-wildcard-import -from dev import * +from lms.envs.dev import * ################################# CELERY ###################################### diff --git a/lms/envs/devgroups/portal.py b/lms/envs/devgroups/portal.py index e972d9f36c3d..c570e555f351 100644 --- a/lms/envs/devgroups/portal.py +++ b/lms/envs/devgroups/portal.py @@ -7,7 +7,7 @@ # want to import all variables from base settings files # pylint: disable=wildcard-import, unused-wildcard-import -from courses import * +from lms.envs.devgroups.courses import * # Move this to a shared file later: for class_id, db_name in CLASSES_TO_DBS.items(): diff --git a/lms/envs/test.py b/lms/envs/test.py index 43f52dc4ea13..e08f7e9bbe9b 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -377,7 +377,7 @@ # Generated checkid_setup request to http://testserver/openid/provider/login/ with assocication {HMAC-SHA1}{51d49995}{s/kRmA==} import openid.oidutil -openid.oidutil.log = lambda message, level = 0: None +openid.oidutil.log = lambda message, level=0: None PLATFORM_NAME = "edX" SITE_NAME = "edx.org" diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index febb533b3314..de76918e1bff 100644 --- a/lms/lib/comment_client/comment.py +++ b/lms/lib/comment_client/comment.py @@ -1,8 +1,8 @@ from .utils import CommentClientRequestError, perform_request from .thread import Thread, _url_for_flag_abuse_thread, _url_for_unflag_abuse_thread -import models -import settings +from comment_client import models +from comment_client import settings class Comment(models.Model): @@ -31,14 +31,16 @@ def thread(self): return Thread(id=self.thread_id, type='thread') @classmethod - def url_for_comments(cls, params={}): + def url_for_comments(cls, params=None): + params = params or {} if params.get('thread_id'): return _url_for_thread_comments(params['thread_id']) else: return _url_for_comment(params['parent_id']) @classmethod - def url(cls, action, params={}): + def url(cls, action, params=None): + params = params or {} if action in ['post']: return cls.url_for_comments(params) else: diff --git a/lms/lib/comment_client/commentable.py b/lms/lib/comment_client/commentable.py index d18a7ccfb739..5c113638b27b 100644 --- a/lms/lib/comment_client/commentable.py +++ b/lms/lib/comment_client/commentable.py @@ -1,6 +1,6 @@ """Provides base Commentable model class""" -import models -import settings +from comment_client import models +from comment_client import settings class Commentable(models.Model): diff --git a/lms/lib/comment_client/models.py b/lms/lib/comment_client/models.py index 555fca883a4d..f9bdf5a79390 100644 --- a/lms/lib/comment_client/models.py +++ b/lms/lib/comment_client/models.py @@ -95,8 +95,8 @@ def _metric_tags(self): return tags @classmethod - def find(cls, id): - return cls(id=id) + def find(cls, identifier): + return cls(id=identifier) def _update_from_response(self, response_data): for k, v in response_data.items(): @@ -155,15 +155,18 @@ def delete(self): self._update_from_response(response) @classmethod - def url_with_id(cls, params={}): + def url_with_id(cls, params=None): + params = params or {} return cls.base_url + '/' + str(params['id']) @classmethod - def url_without_id(cls, params={}): + def url_without_id(cls, params=None): + params = params or {} return cls.base_url @classmethod - def url(cls, action, params={}): + def url(cls, action, params=None): + params = params or {} if cls.base_url is None: raise CommentClientRequestError("Must provide base_url when using default url function") if action not in cls.DEFAULT_ACTIONS: diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 84839b87d0fa..13e735deffa3 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -92,19 +92,21 @@ def search(cls, query_params): return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1), response.get('corrected_text') @classmethod - def url_for_threads(cls, params={}): + def url_for_threads(cls, params=None): + params = params or {} if params.get('commentable_id'): return u"{prefix}/{commentable_id}/threads".format(prefix=settings.PREFIX, commentable_id=params['commentable_id']) else: return u"{prefix}/threads".format(prefix=settings.PREFIX) @classmethod - def url_for_search_threads(cls, params={}): + def url_for_search_threads(cls, params=None): + params = params or {} return "{prefix}/search/threads".format(prefix=settings.PREFIX) @classmethod - def url(cls, action, params={}): - + def url(cls, action, params=None): + params = params or {} if action in ['get_all', 'post']: return cls.url_for_threads(params) elif action == 'search': diff --git a/lms/lib/comment_client/user.py b/lms/lib/comment_client/user.py index 0326dea214c4..d559e70f8b1a 100644 --- a/lms/lib/comment_client/user.py +++ b/lms/lib/comment_client/user.py @@ -83,7 +83,8 @@ def unvote(self, voteable): ) voteable._update_from_response(response) - def active_threads(self, query_params={}): + def active_threads(self, query_params=None): + query_params = query_params or {} if not self.course_id: raise CommentClientRequestError("Must provide course_id when retrieving active threads for the user") url = _url_for_user_active_threads(self.id) @@ -99,7 +100,8 @@ def active_threads(self, query_params={}): ) return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1) - def subscribed_threads(self, query_params={}): + def subscribed_threads(self, query_params=None): + query_params = query_params or {} if not self.course_id: raise CommentClientRequestError("Must provide course_id when retrieving subscribed threads for the user") url = _url_for_user_subscribed_threads(self.id) diff --git a/lms/tests.py b/lms/tests.py index 36c0aa05ae75..88eabd4f2c4c 100644 --- a/lms/tests.py +++ b/lms/tests.py @@ -35,7 +35,14 @@ def test_add_lookup_to_main(self): add_lookup('main', 'external_module', __name__) directories = LOOKUP['main'].directories - self.assertEqual(len([dir for dir in directories if 'external_module' in dir]), 1) + self.assertEqual( + len([ + directory + for directory in directories + if 'external_module' in directory + ]), + 1, + ) # This should not clear the directories list startup.enable_microsites() diff --git a/lms/urls.py b/lms/urls.py index a3b604d756da..544e583c0f25 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -115,16 +115,20 @@ url(r'^course_modes/', include('course_modes.urls')), ) - -js_info_dict = { - 'domain': 'djangojs', - # We need to explicitly include external Django apps that are not in LOCALE_PATHS. - 'packages': ('openassessment',), -} - urlpatterns += ( # Serve catalog of localized strings to be rendered by Javascript - url(r'^jsi18n/$', 'django.views.i18n.javascript_catalog', js_info_dict), + url( + r'^jsi18n/$', + 'django.views.i18n.javascript_catalog', + { + 'domain': 'djangojs', + # We need to explicitly include external Django apps that + # are not in LOCALE_PATHS. + 'packages': ( + 'openassessment', + ), + }, + ), ) # sysadmin dashboard, to see what courses are loaded, to delete & load courses @@ -143,13 +147,22 @@ {'template': '404.html'}, name="404"), ) -# Favicon -favicon_path = microsite.get_value('favicon_path', settings.FAVICON_PATH) -urlpatterns += (( - r'^favicon\.ico$', - 'django.views.generic.simple.redirect_to', - {'url': settings.STATIC_URL + favicon_path} -),) +urlpatterns += ( + # Favicon: give precedence to `microsite` over `settings` + ( + r'^favicon\.ico$', + 'django.views.generic.simple.redirect_to', + { + 'url': "{url_static}{path_favicon}".format( + url_static=settings.STATIC_URL, + path_favicon=microsite.get_value( + 'favicon_path', + settings.FAVICON_PATH, + ), + ), + }, + ), +) # Semi-static views only used by edX, not by themes if not settings.FEATURES["USE_CUSTOM_THEME"]: @@ -537,7 +550,6 @@ urlpatterns += ( url(r'^edinsights_service/', include('edinsights.core.urls')), ) - import edinsights.core.registry # FoldIt views urlpatterns += ( @@ -583,12 +595,8 @@ # in debug mode, allow any template to be rendered (most useful for UX reference templates) urlpatterns += url(r'^template/(?P