Merge lp:~ivle-dev/ivle/marks-fix into lp:ivle

[Matt Giuca]

[Matt Giuca (mgiuca)]

Ready to merge ivle-marks into trunk, completely rewritten. Please review. (I even did your --cutoff feature, Will! Bonus marks for me!)

Commit log as follows:

bin/ivle-marks: Completely rewritten. (Was hopelessly out-of-date, and didn't work at all. Now works on the current database model, and has new features to boot - able to select the semester and cutoff date).

ivle.worksheet.utils: Added calculate_mark, which is from the duplicated code in both bin/ivle-marks and ivle.webapp.tutorial.OfferingView.populate. Also added some extra optional arguments for calculating exercise/worksheet scores from a particular date.

ivle.webapp.tutorial: Call ivle.worksheet.utils.calculate_mark instead of manually calculating (avoid code duplication).

ivle.database: A few new methods on the Subject class, for retrieving offerings.

[lp:~ivle-dev/ivle/marks-fix]

1214. By Matt Giuca on 2009-04-25

ivle.database: Removed Worksheet.get_by_name.
    This method wasn't used anywhere and was invalid (it took a subject name
    which isn't sufficient now that we have offerings). Just getting rid of it
    should do no harm.

[William Grant (wgrant)]

Looks fine to me.

review: Approve

=== modified file 'bin/ivle-marks'
--- bin/ivle-marks	2009-02-26 01:22:17 +0000
+++ bin/ivle-marks	2009-04-24 14:29:12 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 # IVLE - Informatics Virtual Learning Environment
-# Copyright (C) 2007-2008 The University of Melbourne
+# Copyright (C) 2007-2009 The University of Melbourne
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -18,44 +18,25 @@
 
 # Program: Marks
 # Author:  Matt Giuca
-# Date:    17/4/2008
 
 # Script to calculate the marks for all students for a particular subject.
 # Requires root to run.
 
 import sys
 import os
-import re
 import csv
+import datetime
+import optparse
 from xml.dom import minidom
 
+if os.getuid() != 0:
+    print >>sys.stderr, "Must run %s as root." % os.path.basename(sys.argv[0])
+    sys.exit()
+
 import ivle.database
 import ivle.worksheet.utils
 import ivle.conf
 
-if os.getuid() != 0:
-    print >>sys.stderr, "Must run %s as root." % os.path.basename(sys.argv[0])
-    sys.exit()
-
-if len(sys.argv) <= 1:
-    print >>sys.stderr, "Usage: %s subject" % os.path.basename(sys.argv[0])
-    sys.exit()
-
-# Regex for valid identifiers (subject/worksheet names)
-re_ident = re.compile("[0-9A-Za-z_]+")
-
-# This code copy/edited from www/apps/tutorial/__init__.py
-def is_valid_subjname(subject):
-    m = re_ident.match(subject)
-    return m is not None and m.end() == len(subject)
-
-subject = sys.argv[1]
-
-# Subject names must be valid identifiers
-if not is_valid_subjname(subject):
-    print >>sys.stderr, "Invalid subject name: %s." % repr(subject)
-    sys.exit()
-
 def get_userdata(user):
     """
     Given a User object, returns a list of strings for the user data which
@@ -65,102 +46,155 @@
     last_login = (None if user.last_login is None else
                     user.last_login.strftime("%d/%m/%y"))
     return [user.studentid, user.login, user.fullname, last_login]
+
 userdata_header = ["Student ID", "Login", "Full name", "Last login"]
-
-def get_assessable_worksheets(subject):
-    """
-    Given a subject name, returns a list of strings - the worksheet names (not
-    primary key IDs) for all assessable worksheets for that subject.
-    May raise Exceptions, which are fatal.
-    """
-    # NOTE: This code is copy/edited from
-    # www/apps/tutorial/__init__.py:handle_subject_menu
-    # Should be factored out of there.
-
-    # Parse the subject description file
-    # The subject directory must have a file "subject.xml" in it,
-    # or it does not exist (raise exception).
-    try:
-        subjectfile = open(os.path.join(ivle.conf.subjects_base, subject,
-            "subject.xml"))
-    except:
-        raise Exception("Subject %s not found." % repr(subject))
-
-    assessable_worksheets = []
-    # Read in data about the subject
-    subjectdom = minidom.parse(subjectfile)
-    subjectfile.close()
-    # TEMP: All of this is for a temporary XML format, which will later
-    # change.
-    worksheetsdom = subjectdom.documentElement
-    worksheets = []     # List of string IDs
-    for worksheetdom in worksheetsdom.childNodes:
-        if worksheetdom.nodeType == worksheetdom.ELEMENT_NODE:
-            # (Note: assessable will default to False, unless it is explicitly
-            # set to "true").
-            if worksheetdom.getAttribute("assessable") == "true":
-                assessable_worksheets.append(worksheetdom.getAttribute("id"))
-
-    return assessable_worksheets
-
-def get_marks_header(worksheets):
-    """
-    Given a list of strings - the assessable worksheets - returns a new list
-    of strings - the column headings for the marks section of the CSV output.
-    """
-    return worksheets + ["Total %", "Mark"]
-
-def get_marks_user(subject, worksheet_names, user):
-    """
-    Given a subject, a list of strings (the assessable worksheets), and a user
-    object, returns the user's percentage for each worksheet, overall, and
+def get_header(worksheets):
+    """
+    Given a list of Worksheet objects (the assessable worksheets), returns a
+    list of strings -- the column headings for the marks section of the CSV
+    output.
+    """
+    return (userdata_header + [ws.name for ws in worksheets]
+            + ["Total %", "Mark"])
+
+def get_marks_user(worksheets, user, as_of=None):
+    """Gets marks for a particular user for a particular set of worksheets.
+    @param worksheets: List of Worksheet objects to get marks for.
+    @param user: User to get marks for.
+    @param as_of: Optional datetime. If supplied, gets the marks as of as_of.
+    @returns: The user's percentage for each worksheet, overall, and
     their final mark, as a list of strings, in a manner which corresponds to
     the headings produced by get_marks_header.
     """
-    # NOTE: This code is copy/edited from
-    # www/apps/tutorial/__init__.py:handle_subject_menu
-    # Should be factored out of there.
-
     worksheet_pcts = []
     # As we go, calculate the total score for this subject
     # (Assessable worksheets only, mandatory problems only)
     problems_done = 0
     problems_total = 0
 
-    for worksheet_name in worksheet_names:
-        worksheet = ivle.database.Worksheet.get_by_name(store,
-            subject, worksheet_name)
+    for worksheet in worksheets:
         # We simply ignore optional exercises here
         mand_done, mand_total, _, _ = (
-            ivle.worksheet.utils.calculate_score(store, user, worksheet))
-        worksheet_pcts.append(float(mand_done) / mand_total)
+            ivle.worksheet.utils.calculate_score(store, user, worksheet,
+                                                 as_of))
+        if mand_total > 0:
+            worksheet_pcts.append(float(mand_done) / mand_total)
+        else:
+            # Avoid Div0, just give everyone 0 marks if there are none
+            worksheet_pcts.append(0.0)
         problems_done += mand_done
         problems_total += mand_total
-    problems_pct = float(problems_done) / problems_total
-    problems_pct_int = (100 * problems_done) / problems_total
-    # XXX Marks calculation (should be abstracted out of here!)
-    # percent / 16, rounded down, with a maximum mark of 5
-    max_mark = 5
-    mark = min(problems_pct_int / 16, max_mark)
-    return worksheet_pcts + [problems_pct, mark]
+    percent, mark, _ = (
+        ivle.worksheet.utils.calculate_mark(problems_done, problems_total))
+    return worksheet_pcts + [float(percent)/100, mark]
 
-def writeuser(subject, worksheets, user, csvfile):
+def writeuser(worksheets, user, csvfile, cutoff=None):
     userdata = get_userdata(user)
-    marksdata = get_marks_user(subject, worksheets, user)
-    csvfile.writerow(userdata + marksdata)
-
-try:
-    # Get the list of assessable worksheets from the subject.xml file.
-    worksheets = get_assessable_worksheets(subject)
+    marksdata = get_marks_user(worksheets, user, cutoff)
+    data = userdata + marksdata
+    # CSV writer can't handle non-ASCII characters. Encode to UTF-8.
+    data = [unicode(x).encode('utf-8') for x in data]
+    csvfile.writerow(data)
+
+def main(argv=None):
+    global store
+    if argv is None:
+        argv = sys.argv
+
+    usage = """usage: %prog [options] subject
+    (requires root)
+    Reports each student's marks for a given subject offering."""
+
+    # Parse arguments
+    parser = optparse.OptionParser(usage)
+    parser.add_option("-s", "--semester",
+        action="store", dest="semester", metavar="YEAR/SEMESTER",
+        help="Semester of the subject's offering (eg. 2009/1). "
+             "Defaults to the currently active semester.",
+        default=None)
+    parser.add_option("-c", "--cutoff",
+        action="store", dest="cutoff", metavar="DATE",
+        help="Cutoff date (calculate the marks as of this date). "
+             "YYYY-MM-DD H:M:S.",
+        default=None)
+    (options, args) = parser.parse_args(argv[1:])
+
+    if len(args) < 1:
+        parser.print_help()
+        parser.exit()
+
+    subject_name = unicode(args[0])
+
+    if options.semester is None:
+        year, semester = None, None
+    else:
+        try:
+            year, semester = options.semester.split('/')
+            if len(year) == 0 or len(semester) == 0:
+                raise ValueError()
+        except ValueError:
+            parser.error('Invalid semester (must have form "year/semester")')
+
+    if options.cutoff is not None:
+        try:
+            cutoff = datetime.datetime.strptime(options.cutoff,
+                                                "%Y-%m-%d %H:%M:%S")
+        except ValueError:
+            parser.error("Invalid date format: '%s' "
+                         "(must be YYYY-MM-DD H:M:S)." % options.cutoff)
+    else:
+        cutoff = None
+
     store = ivle.database.get_store()
-except Exception, message:
-    print >>sys.stderr, "Error: " + str(message)
-    sys.exit(1)
-
-# Start writing the CSV file - header
-csvfile = csv.writer(sys.stdout)
-csvfile.writerow(userdata_header + get_marks_header(worksheets))
-
-for user in store.find(ivle.database.User).order_by(ivle.database.User.login):
-    writeuser(subject, worksheets, user, csvfile)
-
+
+    # Get the subject from the DB
+    subject = store.find(ivle.database.Subject,
+                     ivle.database.Subject.short_name == subject_name).one()
+    if subject is None:
+        print >>sys.stderr, "No subject with short name '%s'" % subject_name
+        return 1
+
+    # Get the offering from the DB
+    if semester is None:
+        # None specified - get the current offering from the DB
+        offerings = list(subject.active_offerings())
+        if len(offerings) == 0:
+            print >>sys.stderr, ("No active offering for subject '%s'"
+                                 % subject_name)
+            return 1
+        elif len(offerings) > 1:
+            print >>sys.stderr, ("Multiple active offerings for subject '%s':"
+                                 % subject_name)
+            print >>sys.stderr, "Please use one of:"
+            for offering in offerings:
+                print >>sys.stderr, ("    --semester=%s/%s"
+                    % (offering.semester.year, offering.semester.semester))
+            return 1
+        else:
+            offering = offerings[0]
+    else:
+        # Get the offering for the specified semester
+        offering = subject.offering_for_semester(year, semester)
+        if offering is None:
+            print >>sys.stderr, (
+                "No offering for subject '%s' in semester %s/%s"
+                % (subject_name, year, semester))
+            return 1
+
+    # Get the list of assessable worksheets
+    worksheets = offering.worksheets.find(assessable=True)
+
+    # Start writing the CSV file - header
+    csvfile = csv.writer(sys.stdout)
+    csvfile.writerow(get_header(worksheets))
+
+    # Get all users enrolled in this offering
+    users = store.find(ivle.database.User,
+                   ivle.database.User.id == ivle.database.Enrolment.user_id,
+                   offering.id == ivle.database.Enrolment.offering).order_by(
+                        ivle.database.User.login)
+    for user in users:
+        writeuser(worksheets, user, csvfile, cutoff)
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

=== modified file 'ivle/database.py'
--- ivle/database.py	2009-04-07 03:44:43 +0000
+++ ivle/database.py	2009-04-24 12:43:55 +0000
@@ -256,6 +256,20 @@
                 perms.add('edit')
         return perms
 
+    def active_offerings(self):
+        """Return a sequence of currently active offerings for this subject
+        (offerings whose semester.state is "current"). There should be 0 or 1
+        elements in this sequence, but it's possible there are more.
+        """
+        return self.offerings.find(Offering.semester_id == Semester.id,
+                                   Semester.state == u'current')
+
+    def offering_for_semester(self, year, semester):
+        """Get the offering for the given year/semester, or None."""
+        return self.offerings.find(Offering.semester_id == Semester.id,
+                               Semester.year == unicode(year),
+                               Semester.semester == unicode(semester)).one()
+
 class Semester(Storm):
     __storm_table__ = "semester"
 

=== modified file 'ivle/webapp/tutorial/__init__.py'
--- ivle/webapp/tutorial/__init__.py	2009-03-17 01:42:19 +0000
+++ ivle/webapp/tutorial/__init__.py	2009-04-24 13:44:39 +0000
@@ -137,14 +137,10 @@
                 ctx['complete_class'] = "semicomplete"
             else:
                 ctx['complete_class'] = "incomplete"
-            ctx['problems_pct'] = (100 * problems_done) / problems_total
-
-            # We want to display a students mark out of 5. However, they are
-            # allowed to skip 1 in 5 questions and still get 'full marks'.
-            # Hence we divide by 16, essentially making 16 percent worth
-            # 1 star, and 80 or above worth 5.
-            ctx['max_mark'] = 5
-            ctx['mark'] = min(ctx['problems_pct'] / 16, ctx['max_mark'])
+            # Calculate the final percentage and mark for the subject
+            ctx['problems_pct'], ctx['mark'], ctx['max_mark'] = (
+                ivle.worksheet.utils.calculate_mark(
+                    problems_done, problems_total))
 
 class WorksheetView(XHTMLView):
     '''The view of a worksheet with exercises.'''

=== modified file 'ivle/worksheet/utils.py'
--- ivle/worksheet/utils.py	2009-02-26 01:22:17 +0000
+++ ivle/worksheet/utils.py	2009-04-24 14:29:12 +0000
@@ -34,9 +34,13 @@
            'get_exercise_attempts', 'get_exercise_attempt',
           ]
 
-def get_exercise_status(store, user, worksheet_exercise):
+def get_exercise_status(store, user, worksheet_exercise, as_of=None):
     """Given a storm.store, User and Exercise, returns information about
     the user's performance on that problem.
+    @param store: A storm.store
+    @param user: A User.
+    @param worksheet_exercise: An Exercise.
+    @param as_of: Optional datetime. If supplied, gets the status as of as_of.
     Returns a tuple of:
         - A boolean, whether they have successfully passed this exercise.
         - An int, the number of attempts they have made up to and
@@ -48,6 +52,8 @@
     is_relevant = ((ExerciseAttempt.user_id == user.id) &
             (ExerciseAttempt.ws_ex_id == worksheet_exercise.id) &
             (ExerciseAttempt.active == True))
+    if as_of is not None:
+        is_relevant &= ExerciseAttempt.date <= as_of
 
     # Get the first successful active attempt, or None if no success yet.
     # (For this user, for this exercise).
@@ -162,10 +168,14 @@
     saved.date = date
     saved.text = text
 
-def calculate_score(store, user, worksheet):
+def calculate_score(store, user, worksheet, as_of=None):
     """
     Given a storm.store, User, Exercise and Worksheet, calculates a score for
     the user on the given worksheet.
+    @param store: A storm.store
+    @param user: A User.
+    @param worksheet: A Worksheet.
+    @param as_of: Optional datetime. If supplied, gets the score as of as_of.
     Returns a 4-tuple of ints, consisting of:
     (No. mandatory exercises completed,
      Total no. mandatory exercises,
@@ -183,7 +193,7 @@
         worksheet = worksheet_exercise.worksheet
         optional = worksheet_exercise.optional
 
-        done, _ = get_exercise_status(store, user, worksheet_exercise)
+        done, _ = get_exercise_status(store, user, worksheet_exercise, as_of)
         # done is a bool, whether this student has completed that problem
         if optional:
             opt_total += 1
@@ -194,6 +204,33 @@
 
     return mand_done, mand_total, opt_done, opt_total
 
+def calculate_mark(mand_done, mand_total):
+    """Calculate a subject mark, given the result of all worksheets.
+    @param mand_done: The total number of mandatory exercises completed by
+        some student, across all worksheets.
+    @param mand_total: The total number of mandatory exercises across all
+        worksheets in the offering.
+    @return: (percent, mark, mark_total)
+        percent: The percentage of exercises the student has completed, as an
+            integer between 0 and 100 inclusive.
+        mark: The mark the student has received, based on the percentage.
+        mark_total: The total number of marks available (currently hard-coded
+            as 5).
+    """
+    # We want to display a students mark out of 5. However, they are
+    # allowed to skip 1 in 5 questions and still get 'full marks'.
+    # Hence we divide by 16, essentially making 16 percent worth
+    # 1 star, and 80 or above worth 5.
+    if mand_total > 0:
+        percent_int = (100 * mand_done) // mand_total
+    else:
+        # Avoid Div0, just give everyone 0 marks if there are none available
+        percent_int = 0
+    # percent / 16, rounded down, with a maximum mark of 5
+    max_mark = 5
+    mark = min(percent_int // 16, max_mark)
+    return (percent_int, mark, max_mark)
+
 def update_exerciselist(worksheet):
     """Runs through the worksheetstream, generating the appropriate
     WorksheetExercises, and de-activating the old ones."""