From fe0447093c0b32fb2ab00f0ac52079d8c733983a Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 14:00:18 -0400 Subject: [PATCH 1/4] add management command to regrade partial-credit problems affected by get_npoints bug. --- .../management/commands/regrade_partial.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 lms/djangoapps/courseware/management/commands/regrade_partial.py diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py new file mode 100644 index 0000000000..7bfcb16913 --- /dev/null +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -0,0 +1,90 @@ +import json +import logging +from optparse import make_option + +from django.core.management.base import BaseCommand # , CommandError + +from courseware.models import StudentModule +from capa.correctmap import CorrectMap + +# +# This is aimed at fixing a temporary problem encountered where partial credit was awarded for +# code problems, but the resulting score (or grade) was mistakenly set to zero +# because of a bug in CorrectMap.get_npoints(). +# +# The fix here is to recalculate the score/grade based on the partial credit. +# To narrow down the set of problems that might need fixing, the StudentModule +# objects to be checked is filtered down to those: +# +# grade=0.0 (the grade must have been zeroed out) +# created < '2013-03-08 05:19:00' (the problem must have been answered before the fix was installed) +# modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) +# state like '%"npoints": 0.%' (the problem must have some form of partial credit). +# + +log = logging.getLogger(__name__) + +class Command(BaseCommand): + + num_visited = 0 + num_changed = 0 + + option_list = BaseCommand.option_list + ( + make_option('--save', + action='store_true', + dest='save_changes', + default=False, + help='Persist the changes that were encountered. If not set, no changes are saved.'), ) + + def fix_studentmodules(self, save_changes): + modules = StudentModule.objects.filter(# module_type='problem', + modified__gt='2013-03-07 20:18:00', + created__lt='2013-03-08 05:19:00', + state__contains='"npoints": 0.', + grade=0.0) + for module in modules: + self.fix_studentmodule(module, save_changes) + + def fix_studentmodule(self, module, save_changes): + module_state = module.state + if module_state is None: + log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + return + + state_dict = json.loads(module_state) + self.num_visited += 1 + + correct_map = CorrectMap() + if 'correct_map' in state_dict: + correct_map.set_dict(state_dict['correct_map']) + + correct = 0 + for key in correct_map: + correct += correct_map.get_npoints(key) + + if module.grade == correct: + log.info("Grade matches for {type} module {id} for student {student} in course {course_id}".format( + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + elif save_changes: + log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( + module.grade, correct, + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + module.grade = correct + module.save() + self.num_changed += 1 + else: + log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( + module.grade, correct, + **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + self.num_changed += 1 + + def handle(self, **options): + save_changes = 'save_changes' in options and options['save_changes'] + + log.info("Starting run: save_changes = {0}".format(save_changes)) + + self.fix_studentmodules(save_changes) + + log.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) + From 6aed059d5ff70e18335cf7e12e3a5812108060a4 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 14:55:27 -0400 Subject: [PATCH 2/4] Change date and remove filtering by grade. Add check for student_answers. --- .../management/commands/regrade_partial.py | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py index 7bfcb16913..b997cff674 100644 --- a/lms/djangoapps/courseware/management/commands/regrade_partial.py +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -16,8 +16,7 @@ from capa.correctmap import CorrectMap # To narrow down the set of problems that might need fixing, the StudentModule # objects to be checked is filtered down to those: # -# grade=0.0 (the grade must have been zeroed out) -# created < '2013-03-08 05:19:00' (the problem must have been answered before the fix was installed) +# created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, on Prod and Edge) # modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) # state like '%"npoints": 0.%' (the problem must have some form of partial credit). # @@ -39,15 +38,16 @@ class Command(BaseCommand): def fix_studentmodules(self, save_changes): modules = StudentModule.objects.filter(# module_type='problem', modified__gt='2013-03-07 20:18:00', - created__lt='2013-03-08 05:19:00', - state__contains='"npoints": 0.', - grade=0.0) + created__lt='2013-03-08 15:45:00', + state__contains='"npoints": 0.') + for module in modules: - self.fix_studentmodule(module, save_changes) + self.fix_studentmodule_grade(module, save_changes) - def fix_studentmodule(self, module, save_changes): + def fix_studentmodule_grade(self, module, save_changes): module_state = module.state if module_state is None: + # not likely, since we filter on it. But in general... log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) return @@ -55,32 +55,50 @@ class Command(BaseCommand): state_dict = json.loads(module_state) self.num_visited += 1 + # LoncapaProblem.get_score() checks student_answers -- if there are none, we will return a grade of 0 + # Check that this is the case, but do so sooner, before we do any of the other grading work. + student_answers = state_dict['student_answers'] + if (not student_answers) or len(student_answers) == 0: + # we should not have a grade here: + if module.grade != 0: + log.error("No answer found but grade {grade} exists for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + else: + log.debug("No answer and no grade found for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + return + + # load into a CorrectMap, as done in LoncapaProblem.__init__(): correct_map = CorrectMap() if 'correct_map' in state_dict: correct_map.set_dict(state_dict['correct_map']) + # calculate score the way LoncapaProblem.get_score() works, by deferring to CorrectMap's get_npoints implementation. correct = 0 for key in correct_map: correct += correct_map.get_npoints(key) if module.grade == correct: - log.info("Grade matches for {type} module {id} for student {student} in course {course_id}".format( - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + # nothing to change + log.debug("Grade matches for {type} module {id} for student {student} in course {course_id}".format( + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) elif save_changes: + # make the change log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( module.grade, correct, - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) module.grade = correct module.save() self.num_changed += 1 else: + # don't make the change, but log that the change would be made log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( module.grade, correct, - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) self.num_changed += 1 def handle(self, **options): - save_changes = 'save_changes' in options and options['save_changes'] + save_changes = options['save_changes'] log.info("Starting run: save_changes = {0}".format(save_changes)) From 47e708e713735fc94cb38f2096057d36cc7410e3 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 12 Mar 2013 15:49:48 -0400 Subject: [PATCH 3/4] fix pylint and pep8 warnings --- .../management/commands/regrade_partial.py | 104 ++++++++++-------- 1 file changed, 59 insertions(+), 45 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/regrade_partial.py b/lms/djangoapps/courseware/management/commands/regrade_partial.py index b997cff674..c24f84ddfc 100644 --- a/lms/djangoapps/courseware/management/commands/regrade_partial.py +++ b/lms/djangoapps/courseware/management/commands/regrade_partial.py @@ -1,33 +1,36 @@ +''' +This is a one-off command aimed at fixing a temporary problem encountered where partial credit was awarded for +code problems, but the resulting score (or grade) was mistakenly set to zero because of a bug in +CorrectMap.get_npoints(). +''' + import json import logging from optparse import make_option -from django.core.management.base import BaseCommand # , CommandError +from django.core.management.base import BaseCommand from courseware.models import StudentModule from capa.correctmap import CorrectMap -# -# This is aimed at fixing a temporary problem encountered where partial credit was awarded for -# code problems, but the resulting score (or grade) was mistakenly set to zero -# because of a bug in CorrectMap.get_npoints(). -# -# The fix here is to recalculate the score/grade based on the partial credit. -# To narrow down the set of problems that might need fixing, the StudentModule -# objects to be checked is filtered down to those: -# -# created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, on Prod and Edge) -# modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) -# state like '%"npoints": 0.%' (the problem must have some form of partial credit). -# +LOG = logging.getLogger(__name__) -log = logging.getLogger(__name__) class Command(BaseCommand): - + ''' + The fix here is to recalculate the score/grade based on the partial credit. + To narrow down the set of problems that might need fixing, the StudentModule + objects to be checked is filtered down to those: + + created < '2013-03-08 15:45:00' (the problem must have been answered before the fix was installed, + on Prod and Edge) + modified > '2013-03-07 20:18:00' (the problem must have been visited after the bug was introduced) + state like '%"npoints": 0.%' (the problem must have some form of partial credit). + ''' + num_visited = 0 num_changed = 0 - + option_list = BaseCommand.option_list + ( make_option('--save', action='store_true', @@ -36,73 +39,84 @@ class Command(BaseCommand): help='Persist the changes that were encountered. If not set, no changes are saved.'), ) def fix_studentmodules(self, save_changes): - modules = StudentModule.objects.filter(# module_type='problem', - modified__gt='2013-03-07 20:18:00', + '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' + modules = StudentModule.objects.filter(modified__gt='2013-03-07 20:18:00', created__lt='2013-03-08 15:45:00', state__contains='"npoints": 0.') for module in modules: self.fix_studentmodule_grade(module, save_changes) - + def fix_studentmodule_grade(self, module, save_changes): + ''' Fix the grade assigned to a StudentModule''' module_state = module.state if module_state is None: # not likely, since we filter on it. But in general... - log.info("No state found for {type} module {id} for student {student} in course {course_id}".format( - **{'type':module.module_type, 'id':module.module_state_key, 'student':module.student.username, 'course_id':module.course_id})) + LOG.info("No state found for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) return - + state_dict = json.loads(module_state) self.num_visited += 1 - + # LoncapaProblem.get_score() checks student_answers -- if there are none, we will return a grade of 0 # Check that this is the case, but do so sooner, before we do any of the other grading work. student_answers = state_dict['student_answers'] if (not student_answers) or len(student_answers) == 0: # we should not have a grade here: if module.grade != 0: - log.error("No answer found but grade {grade} exists for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.error("No answer found but grade {grade} exists for {type} module {id} for student {student} " + "in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) else: - log.debug("No answer and no grade found for {type} module {id} for student {student} in course {course_id}".format(grade=module.grade, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.debug("No answer and no grade found for {type} module {id} for student {student} " + "in course {course_id}".format(grade=module.grade, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) return # load into a CorrectMap, as done in LoncapaProblem.__init__(): correct_map = CorrectMap() if 'correct_map' in state_dict: correct_map.set_dict(state_dict['correct_map']) - - # calculate score the way LoncapaProblem.get_score() works, by deferring to CorrectMap's get_npoints implementation. + + # calculate score the way LoncapaProblem.get_score() works, by deferring to + # CorrectMap's get_npoints implementation. correct = 0 for key in correct_map: correct += correct_map.get_npoints(key) - + if module.grade == correct: # nothing to change - log.debug("Grade matches for {type} module {id} for student {student} in course {course_id}".format( - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.debug("Grade matches for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) elif save_changes: # make the change - log.info("Grade changing from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( - module.grade, correct, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.info("Grade changing from {0} to {1} for {type} module {id} for student {student} " + "in course {course_id}".format(module.grade, correct, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) module.grade = correct module.save() self.num_changed += 1 else: # don't make the change, but log that the change would be made - log.info("Grade would change from {0} to {1} for {type} module {id} for student {student} in course {course_id}".format( - module.grade, correct, - type=module.module_type, id=module.module_state_key, student=module.student.username, course_id=module.course_id)) + LOG.info("Grade would change from {0} to {1} for {type} module {id} for student {student} " + "in course {course_id}".format(module.grade, correct, + type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) self.num_changed += 1 - + def handle(self, **options): + '''Handle management command request''' + save_changes = options['save_changes'] - log.info("Starting run: save_changes = {0}".format(save_changes)) - + LOG.info("Starting run: save_changes = {0}".format(save_changes)) + self.fix_studentmodules(save_changes) - - log.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) - + + LOG.info("Finished run: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) From 47420fe7c5b29d9aa5b3d0724495f70b73c36a4f Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 12 Mar 2013 12:41:04 -0400 Subject: [PATCH 4/4] Make sure that test-requirements.txt doesn't install versions of libraries that requirements.txt has pinned, and cache pip downloads --- jenkins/test.sh | 5 ++- requirements.txt | 94 +++++++++++++++++++++---------------------- test-requirements.txt | 3 -- 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/jenkins/test.sh b/jenkins/test.sh index 3a572c9808..f8ffab29fc 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -32,10 +32,11 @@ if [ ! -d /mnt/virtualenvs/"$JOB_NAME" ]; then virtualenv /mnt/virtualenvs/"$JOB_NAME" fi +export PIP_DOWNLOAD_CACHE=/mnt/pip-cache + source /mnt/virtualenvs/"$JOB_NAME"/bin/activate pip install -q -r pre-requirements.txt -pip install -q -r test-requirements.txt -yes w | pip install -q -r requirements.txt +yes w | pip install -q -r test-requirements.txt -r requirements.txt rake clobber rake pep8 diff --git a/requirements.txt b/requirements.txt index 204ec85889..3dc732e013 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,61 +1,61 @@ -django==1.4.3 -pip -numpy==1.6.2 -scipy==0.11.0 -Markdown==2.2.1 -pygments==1.5 -lxml==3.0.1 -boto==2.6.0 -mako==0.7.3 -python-memcached==1.48 -python-openid==2.2.5 -path.py -django_debug_toolbar -fs==0.4.0 -beautifulsoup==3.2.1 +-r repo-requirements.txt beautifulsoup4==4.1.3 -feedparser==5.1.3 -requests==0.14.2 -http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz -newrelic==1.8.0.13 -glob2==0.3 -pymongo==2.4.1 -django_nose==1.1 -nosexcover==1.0.7 -rednose==0.3.3 -GitPython==0.3.2.RC1 -mock==0.8.0 -PyYAML==3.10 -South==0.7.6 -pytz==2012h +beautifulsoup==3.2.1 +boto==2.6.0 django-celery==3.0.11 django-countries==1.5 -django-kombu==0.9.4 +django-debug-toolbar-mongo django-followit==0.0.3 django-jasmine==0.3.2 django-keyedcache==1.4-6 +django-kombu==0.9.4 django-mako==0.1.5pre django-masquerade==0.1.6 +django-mptt==0.5.5 django-openid-auth==0.4 django-robots==0.9.1 +django-sekizai==0.6.1 django-ses==0.4.1 django-storages==1.1.5 django-threaded-multihost==1.4-1 -django-sekizai==0.6.1 -django-mptt==0.5.5 -sorl-thumbnail==11.12 -networkx==1.7 -pygraphviz==1.1 --r repo-requirements.txt -nltk==2.0.4 -django-debug-toolbar-mongo -dogstatsd-python==0.2.1 -MySQL-python==1.2.4c1 -sphinx==1.1.3 -factory_boy -Shapely==1.2.16 -ipython==0.13.1 -xmltodict==0.4.1 -paramiko==1.9.0 -Pillow==1.7.8 +django==1.4.3 +django_debug_toolbar +django_nose==1.1 dogapi==1.2.1 +dogstatsd-python==0.2.1 +factory_boy +feedparser==5.1.3 +fs==0.4.0 +GitPython==0.3.2.RC1 +glob2==0.3 +http://sympy.googlecode.com/files/sympy-0.7.1.tar.gz +ipython==0.13.1 +lxml==3.0.1 +mako==0.7.3 +Markdown==2.2.1 +mock==0.8.0 +MySQL-python==1.2.4c1 +networkx==1.7 +newrelic==1.8.0.13 +nltk==2.0.4 +nosexcover==1.0.7 +numpy==1.6.2 +paramiko==1.9.0 +path.py +Pillow==1.7.8 +pip +pygments==1.5 +pygraphviz==1.1 +pymongo==2.4.1 +python-memcached==1.48 +python-openid==2.2.5 +pytz==2012h +PyYAML==3.10 +rednose==0.3.3 +requests==0.14.2 +scipy==0.11.0 +Shapely==1.2.16 +sorl-thumbnail==11.12 +South==0.7.6 +sphinx==1.1.3 +xmltodict==0.4.1 diff --git a/test-requirements.txt b/test-requirements.txt index 4706e239a5..6c3322acbf 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,10 +1,7 @@ -django-nose coverage -nosexcover pylint pep8 lettuce selenium -factory_boy splinter