From 74866a38b00c2ce5a593dc509dcb23f22febaaf0 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 29 May 2013 12:55:11 -0400 Subject: [PATCH 01/55] Move parseActions and statics out of evaluator() --- common/lib/calc/calc.py | 144 ++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 71 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 2ee82e2fb4..0ab02e413b 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -37,16 +37,33 @@ default_variables = {'j': numpy.complex(0, 1), 'q': scipy.constants.e } + +ops = {"^": operator.pow, + "*": operator.mul, + "/": operator.truediv, + "+": operator.add, + "-": operator.sub, +} +# We eliminated extreme ones, since they're rarely used, and potentially +# confusing. They may also conflict with variables if we ever allow e.g. +# 5R instead of 5*R +suffixes = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, + 'T': 1e12, # 'P':1e15,'E':1e18,'Z':1e21,'Y':1e24, + 'c': 1e-2, 'm': 1e-3, 'u': 1e-6, + 'n': 1e-9, 'p': 1e-12} # ,'f':1e-15,'a':1e-18,'z':1e-21,'y':1e-24} + log = logging.getLogger("mitx.courseware.capa") class UndefinedVariable(Exception): - def raiseself(self): - ''' Helper so we can use inside of a lambda ''' - raise self + pass + # unused for now + # def raiseself(self): + # ''' Helper so we can use inside of a lambda ''' + # raise self -general_whitespace = re.compile('[^\w]+') +general_whitespace = re.compile('[^\\w]+') def check_variables(string, variables): @@ -65,13 +82,61 @@ def check_variables(string, variables): for v in possible_variables: if len(v) == 0: continue - if v[0] <= '9' and '0' <= 'v': # Skip things that begin with numbers + if v[0] <= '9' and '0' <= v: # Skip things that begin with numbers continue if v not in variables: bad_variables.append(v) if len(bad_variables) > 0: raise UndefinedVariable(' '.join(bad_variables)) +def lower_dict(d): + return dict([(k.lower(), d[k]) for k in d]) + +def super_float(text): + ''' Like float, but with si extensions. 1k goes to 1000''' + if text[-1] in suffixes: + return float(text[:-1]) * suffixes[text[-1]] + else: + return float(text) + +def number_parse_action(x): # [ '7' ] -> [ 7 ] + return [super_float("".join(x))] + +def exp_parse_action(x): # [ 2 ^ 3 ^ 2 ] -> 512 + x = [e for e in x if isinstance(e, numbers.Number)] # Ignore ^ + x.reverse() + x = reduce(lambda a, b: b ** a, x) + return x + +def parallel(x): # Parallel resistors [ 1 2 ] => 2/3 + # convert from pyparsing.ParseResults, which doesn't support '0 in x' + x = list(x) + if len(x) == 1: + return x[0] + if 0 in x: + return float('nan') + x = [1. / e for e in x if isinstance(e, numbers.Number)] # Ignore || + return 1. / sum(x) + +def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 + total = 0.0 + op = ops['+'] + for e in x: + if e in set('+-'): + op = ops[e] + else: + total = op(total, e) + return total + +def prod_parse_action(x): # [ 1 * 2 / 3 ] => 0.66 + prod = 1.0 + op = ops['*'] + for e in x: + if e in set('*/'): + op = ops[e] + else: + prod = op(prod, e) + return prod def evaluator(variables, functions, string, cs=False): ''' @@ -86,12 +151,12 @@ def evaluator(variables, functions, string, cs=False): # log.debug("functions: {0}".format(functions)) # log.debug("string: {0}".format(string)) - def lower_dict(d): - return dict([(k.lower(), d[k]) for k in d]) - all_variables = copy.copy(default_variables) all_functions = copy.copy(default_functions) + def func_parse_action(x): + return [all_functions[x[0]](x[1])] + if not cs: all_variables = lower_dict(all_variables) all_functions = lower_dict(all_functions) @@ -113,69 +178,6 @@ def evaluator(variables, functions, string, cs=False): if string.strip() == "": return float('nan') - ops = {"^": operator.pow, - "*": operator.mul, - "/": operator.truediv, - "+": operator.add, - "-": operator.sub, - } - # We eliminated extreme ones, since they're rarely used, and potentially - # confusing. They may also conflict with variables if we ever allow e.g. - # 5R instead of 5*R - suffixes = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, - 'T': 1e12, # 'P':1e15,'E':1e18,'Z':1e21,'Y':1e24, - 'c': 1e-2, 'm': 1e-3, 'u': 1e-6, - 'n': 1e-9, 'p': 1e-12} # ,'f':1e-15,'a':1e-18,'z':1e-21,'y':1e-24} - - def super_float(text): - ''' Like float, but with si extensions. 1k goes to 1000''' - if text[-1] in suffixes: - return float(text[:-1]) * suffixes[text[-1]] - else: - return float(text) - - def number_parse_action(x): # [ '7' ] -> [ 7 ] - return [super_float("".join(x))] - - def exp_parse_action(x): # [ 2 ^ 3 ^ 2 ] -> 512 - x = [e for e in x if isinstance(e, numbers.Number)] # Ignore ^ - x.reverse() - x = reduce(lambda a, b: b ** a, x) - return x - - def parallel(x): # Parallel resistors [ 1 2 ] => 2/3 - # convert from pyparsing.ParseResults, which doesn't support '0 in x' - x = list(x) - if len(x) == 1: - return x[0] - if 0 in x: - return float('nan') - x = [1. / e for e in x if isinstance(e, numbers.Number)] # Ignore || - return 1. / sum(x) - - def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 - total = 0.0 - op = ops['+'] - for e in x: - if e in set('+-'): - op = ops[e] - else: - total = op(total, e) - return total - - def prod_parse_action(x): # [ 1 * 2 / 3 ] => 0.66 - prod = 1.0 - op = ops['*'] - for e in x: - if e in set('*/'): - op = ops[e] - else: - prod = op(prod, e) - return prod - - def func_parse_action(x): - return [all_functions[x[0]](x[1])] - # SI suffixes and percent number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch()) (dot, minus, plus, times, div, lpar, rpar, exp) = map(Literal, ".-+*/()^") From ed45c505a39cf3a8aa094ee6c64591da1c604773 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 29 May 2013 12:55:51 -0400 Subject: [PATCH 02/55] Simpler pyparsing --- common/lib/calc/calc.py | 48 +++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 0ab02e413b..64053d6ca5 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -8,11 +8,11 @@ import numpy import numbers import scipy.constants -from pyparsing import Word, alphas, nums, oneOf, Literal -from pyparsing import ZeroOrMore, OneOrMore, StringStart -from pyparsing import StringEnd, Optional, Forward -from pyparsing import CaselessLiteral, Group, StringEnd -from pyparsing import NoMatch, stringEnd, alphanums +from pyparsing import Word, nums, Literal +from pyparsing import ZeroOrMore, MatchFirst +from pyparsing import Optional, Forward +from pyparsing import CaselessLiteral +from pyparsing import NoMatch, stringEnd, Suppress, Combine default_functions = {'sin': numpy.sin, 'cos': numpy.cos, @@ -179,17 +179,19 @@ def evaluator(variables, functions, string, cs=False): return float('nan') # SI suffixes and percent - number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch()) - (dot, minus, plus, times, div, lpar, rpar, exp) = map(Literal, ".-+*/()^") + number_suffix = MatchFirst([Literal(k) for k in suffixes.keys()]) + plus_minus = Literal('+') | Literal('-') + times_div = Literal('*') | Literal('/') number_part = Word(nums) # 0.33 or 7 or .34 or 16. inner_number = (number_part + Optional("." + Optional(number_part))) | ("." + number_part) + inner_number = Combine(inner_number) # by default pyparsing allows spaces between tokens--this prevents that # 0.33k or -17 - number = (Optional(minus | plus) + inner_number - + Optional(CaselessLiteral("E") + Optional((plus | minus)) + number_part) + number = (inner_number + + Optional(CaselessLiteral("E") + Optional(plus_minus) + number_part) + Optional(number_suffix)) number = number.setParseAction(number_parse_action) # Convert to number @@ -197,40 +199,34 @@ def evaluator(variables, functions, string, cs=False): expr = Forward() factor = Forward() - def sreduce(f, l): - ''' Same as reduce, but handle len 1 and len 0 lists sensibly ''' - if len(l) == 0: - return NoMatch() - if len(l) == 1: - return l[0] - return reduce(f, l) - # Handle variables passed in. E.g. if we have {'R':0.5}, we make the substitution. # Special case for no variables because of how we understand PyParsing is put together if len(all_variables) > 0: # We sort the list so that var names (like "e2") match before # mathematical constants (like "e"). This is kind of a hack. all_variables_keys = sorted(all_variables.keys(), key=len, reverse=True) - varnames = sreduce(lambda x, y: x | y, map(lambda x: CasedLiteral(x), all_variables_keys)) - varnames.setParseAction(lambda x: map(lambda y: all_variables[y], x)) + literal_all_vars = [CasedLiteral(k) for k in all_variables_keys] + varnames = MatchFirst(literal_all_vars) + varnames.setParseAction(lambda x: [all_variables[k] for k in x]) else: varnames = NoMatch() # Same thing for functions. if len(all_functions) > 0: - funcnames = sreduce(lambda x, y: x | y, - map(lambda x: CasedLiteral(x), all_functions.keys())) - function = funcnames + lpar.suppress() + expr + rpar.suppress() + funcnames = MatchFirst([CasedLiteral(k) for k in all_functions.keys()]) + function = funcnames + Suppress("(") + expr + Suppress(")") function.setParseAction(func_parse_action) else: function = NoMatch() - atom = number | function | varnames | lpar + expr + rpar - factor << (atom + ZeroOrMore(exp + atom)).setParseAction(exp_parse_action) # 7^6 + atom = number | function | varnames | Suppress("(") + expr + Suppress(")") + + # Do the following in the correct order to preserve order of operation + factor << (atom + ZeroOrMore("^" + atom)).setParseAction(exp_parse_action) # 7^6 paritem = factor + ZeroOrMore(Literal('||') + factor) # 5k || 4k paritem = paritem.setParseAction(parallel) - term = paritem + ZeroOrMore((times | div) + paritem) # 7 * 5 / 4 - 3 + term = paritem + ZeroOrMore(times_div + paritem) # 7 * 5 / 4 - 3 term = term.setParseAction(prod_parse_action) - expr << Optional((plus | minus)) + term + ZeroOrMore((plus | minus) + term) # -5 + 4 - 3 + expr << Optional(plus_minus) + term + ZeroOrMore(plus_minus + term) # -5 + 4 - 3 expr = expr.setParseAction(sum_parse_action) return (expr + stringEnd).parseString(string)[0] From 72d149caae1c5cd3909b59e850d94cb8ffc95c59 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 29 May 2013 13:25:48 -0400 Subject: [PATCH 03/55] Add docstrings and comments --- common/lib/calc/calc.py | 81 +++++++++++++++++++++++---- common/lib/capa/capa/responsetypes.py | 1 + 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 64053d6ca5..5d0aeb3fd1 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -1,3 +1,9 @@ +""" +Parser and evaluator for FormulaResponse and NumericalResponse + +Uses pyparsing to parse. Main function as of now is evaluator(). +""" + import copy import logging import math @@ -56,6 +62,10 @@ log = logging.getLogger("mitx.courseware.capa") class UndefinedVariable(Exception): + """ + Used to indicate the student input of a variable, which was unused by the + instructor. + """ pass # unused for now # def raiseself(self): @@ -67,7 +77,8 @@ general_whitespace = re.compile('[^\\w]+') def check_variables(string, variables): - '''Confirm the only variables in string are defined. + """ + Confirm the only variables in string are defined. Pyparsing uses a left-to-right parser, which makes the more elegant approach pretty hopeless. @@ -76,7 +87,7 @@ def check_variables(string, variables): undefined_variable = achar + Word(alphanums) undefined_variable.setParseAction(lambda x:UndefinedVariable("".join(x)).raiseself()) varnames = varnames | undefined_variable - ''' + """ possible_variables = re.split(general_whitespace, string) # List of all alnums in string bad_variables = list() for v in possible_variables: @@ -90,26 +101,59 @@ def check_variables(string, variables): raise UndefinedVariable(' '.join(bad_variables)) def lower_dict(d): + """ + takes each key in the dict and makes it lowercase, still mapping to the + same value. + + keep in mind that it is possible (but not useful?) to define different + variables that have the same lowercase representation. It would be hard to + tell which is used in the final dict and which isn't. + """ return dict([(k.lower(), d[k]) for k in d]) +# The following few functions define parse actions, which are run on lists of +# results from each parse component. They convert the strings and (previously +# calculated) numbers into the number that component represents. + def super_float(text): - ''' Like float, but with si extensions. 1k goes to 1000''' + """ + Like float, but with si extensions. 1k goes to 1000 + """ if text[-1] in suffixes: return float(text[:-1]) * suffixes[text[-1]] else: return float(text) -def number_parse_action(x): # [ '7' ] -> [ 7 ] +def number_parse_action(x): + """ + Create a float out of its string parts + + e.g. [ '7', '.', '13' ] -> [ 7.13 ] + Calls super_float above + """ return [super_float("".join(x))] -def exp_parse_action(x): # [ 2 ^ 3 ^ 2 ] -> 512 +def exp_parse_action(x): + """ + Take a list of numbers and exponentiate them, right to left + + e.g. [ 3, 2, 3 ] (which is 3^2^3 = 3^(2^3)) -> 6561 + """ x = [e for e in x if isinstance(e, numbers.Number)] # Ignore ^ x.reverse() x = reduce(lambda a, b: b ** a, x) return x -def parallel(x): # Parallel resistors [ 1 2 ] => 2/3 - # convert from pyparsing.ParseResults, which doesn't support '0 in x' +def parallel(x): + """ + Compute numbers according to the parallel resistors operator + + BTW it is commutative. Its formula is given by + out = 1 / (1/in1 + 1/in2 + ...) + e.g. [ 1, 2 ] => 2/3 + + Return NaN if there is a zero among the inputs + """ x = list(x) if len(x) == 1: return x[0] @@ -119,6 +163,13 @@ def parallel(x): # Parallel resistors [ 1 2 ] => 2/3 return 1. / sum(x) def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 + """ + Add the inputs + + [ 1, '+', 2, '-', 3 ] -> 0 + + Allow a leading + or - + """ total = 0.0 op = ops['+'] for e in x: @@ -129,6 +180,11 @@ def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 return total def prod_parse_action(x): # [ 1 * 2 / 3 ] => 0.66 + """ + Multiply the inputs + + [ 1, '*', 2, '/', 3 ] => 0.66 + """ prod = 1.0 op = ops['*'] for e in x: @@ -139,14 +195,13 @@ def prod_parse_action(x): # [ 1 * 2 / 3 ] => 0.66 return prod def evaluator(variables, functions, string, cs=False): - ''' + """ Evaluate an expression. Variables are passed as a dictionary from string to value. Unary functions are passed as a dictionary from string to function. Variables must be floats. cs: Case sensitive - TODO: Fix it so we can pass integers and complex numbers in variables dict - ''' + """ # log.debug("variables: {0}".format(variables)) # log.debug("functions: {0}".format(functions)) # log.debug("string: {0}".format(string)) @@ -187,7 +242,8 @@ def evaluator(variables, functions, string, cs=False): # 0.33 or 7 or .34 or 16. inner_number = (number_part + Optional("." + Optional(number_part))) | ("." + number_part) - inner_number = Combine(inner_number) # by default pyparsing allows spaces between tokens--this prevents that + # by default pyparsing allows spaces between tokens--Combine prevents that + inner_number = Combine(inner_number) # 0.33k or -17 number = (inner_number @@ -209,6 +265,8 @@ def evaluator(variables, functions, string, cs=False): varnames = MatchFirst(literal_all_vars) varnames.setParseAction(lambda x: [all_variables[k] for k in x]) else: + # all_variables includes DEFAULT_VARIABLES, which isn't empty + # this is unreachable. Get rid of it? varnames = NoMatch() # Same thing for functions. @@ -217,6 +275,7 @@ def evaluator(variables, functions, string, cs=False): function = funcnames + Suppress("(") + expr + Suppress(")") function.setParseAction(func_parse_action) else: + # see note above (this is unreachable) function = NoMatch() atom = number | function | varnames | Suppress("(") + expr + Suppress(")") diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 0fa50079de..314d01e7e8 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -1717,6 +1717,7 @@ class FormulaResponse(LoncapaResponse): student_variables = dict() # ranges give numerical ranges for testing for var in ranges: + # TODO: allow specified ranges (i.e. integers and complex numbers) for random variables value = random.uniform(*ranges[var]) instructor_variables[str(var)] = value student_variables[str(var)] = value From a85a7f71df6c0bc889b2d5cbe40926b3663d375e Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 29 May 2013 13:34:58 -0400 Subject: [PATCH 04/55] Rename variables; get rid of OPS --- common/lib/calc/calc.py | 170 ++++++++++++++++++++-------------------- 1 file changed, 87 insertions(+), 83 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 5d0aeb3fd1..f862b41542 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -11,16 +11,15 @@ import operator import re import numpy -import numbers import scipy.constants -from pyparsing import Word, nums, Literal -from pyparsing import ZeroOrMore, MatchFirst -from pyparsing import Optional, Forward -from pyparsing import CaselessLiteral -from pyparsing import NoMatch, stringEnd, Suppress, Combine +from pyparsing import (Word, nums, Literal, + ZeroOrMore, MatchFirst, + Optional, Forward, + CaselessLiteral, + NoMatch, stringEnd, Suppress, Combine) -default_functions = {'sin': numpy.sin, +DEFAULT_FUNCTIONS = {'sin': numpy.sin, 'cos': numpy.cos, 'tan': numpy.tan, 'sqrt': numpy.sqrt, @@ -34,7 +33,7 @@ default_functions = {'sin': numpy.sin, 'fact': math.factorial, 'factorial': math.factorial } -default_variables = {'j': numpy.complex(0, 1), +DEFAULT_VARIABLES = {'j': numpy.complex(0, 1), 'e': numpy.e, 'pi': numpy.pi, 'k': scipy.constants.k, @@ -43,22 +42,15 @@ default_variables = {'j': numpy.complex(0, 1), 'q': scipy.constants.e } - -ops = {"^": operator.pow, - "*": operator.mul, - "/": operator.truediv, - "+": operator.add, - "-": operator.sub, -} # We eliminated extreme ones, since they're rarely used, and potentially # confusing. They may also conflict with variables if we ever allow e.g. # 5R instead of 5*R -suffixes = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, +SUFFIXES = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, 'T': 1e12, # 'P':1e15,'E':1e18,'Z':1e21,'Y':1e24, 'c': 1e-2, 'm': 1e-3, 'u': 1e-6, 'n': 1e-9, 'p': 1e-12} # ,'f':1e-15,'a':1e-18,'z':1e-21,'y':1e-24} -log = logging.getLogger("mitx.courseware.capa") +LOG = logging.getLogger("mitx.courseware.capa") class UndefinedVariable(Exception): @@ -73,13 +65,12 @@ class UndefinedVariable(Exception): # raise self -general_whitespace = re.compile('[^\\w]+') - - def check_variables(string, variables): """ Confirm the only variables in string are defined. + Otherwise, raise an UndefinedVariable containing all bad variables. + Pyparsing uses a left-to-right parser, which makes the more elegant approach pretty hopeless. @@ -88,19 +79,22 @@ def check_variables(string, variables): undefined_variable.setParseAction(lambda x:UndefinedVariable("".join(x)).raiseself()) varnames = varnames | undefined_variable """ - possible_variables = re.split(general_whitespace, string) # List of all alnums in string + general_whitespace = re.compile('[^\\w]+') + # List of all alnums in string + possible_variables = re.split(general_whitespace, string) bad_variables = list() - for v in possible_variables: - if len(v) == 0: + for var in possible_variables: + if len(var) == 0: continue - if v[0] <= '9' and '0' <= v: # Skip things that begin with numbers + if var[0] <= '9' and '0' <= var: # Skip things that begin with numbers continue - if v not in variables: - bad_variables.append(v) + if var not in variables: + bad_variables.append(var) if len(bad_variables) > 0: raise UndefinedVariable(' '.join(bad_variables)) -def lower_dict(d): + +def lower_dict(input_dict): """ takes each key in the dict and makes it lowercase, still mapping to the same value. @@ -109,7 +103,8 @@ def lower_dict(d): variables that have the same lowercase representation. It would be hard to tell which is used in the final dict and which isn't. """ - return dict([(k.lower(), d[k]) for k in d]) + return dict([(k.lower(), input_dict[k]) for k in input_dict]) + # The following few functions define parse actions, which are run on lists of # results from each parse component. They convert the strings and (previously @@ -119,32 +114,37 @@ def super_float(text): """ Like float, but with si extensions. 1k goes to 1000 """ - if text[-1] in suffixes: - return float(text[:-1]) * suffixes[text[-1]] + if text[-1] in SUFFIXES: + return float(text[:-1]) * SUFFIXES[text[-1]] else: return float(text) -def number_parse_action(x): + +def number_parse_action(parse_result): """ Create a float out of its string parts e.g. [ '7', '.', '13' ] -> [ 7.13 ] Calls super_float above """ - return [super_float("".join(x))] + return super_float("".join(parse_result)) -def exp_parse_action(x): + +def exp_parse_action(parse_result): """ Take a list of numbers and exponentiate them, right to left e.g. [ 3, 2, 3 ] (which is 3^2^3 = 3^(2^3)) -> 6561 """ - x = [e for e in x if isinstance(e, numbers.Number)] # Ignore ^ - x.reverse() - x = reduce(lambda a, b: b ** a, x) - return x + # pyparsing.ParseResults doesn't play well with reverse() + parse_result = parse_result.asList() + parse_result.reverse() + # the result of an exponentiation is called a power + power = reduce(lambda a, b: b ** a, parse_result) + return power -def parallel(x): + +def parallel(parse_result): """ Compute numbers according to the parallel resistors operator @@ -154,15 +154,17 @@ def parallel(x): Return NaN if there is a zero among the inputs """ - x = list(x) - if len(x) == 1: - return x[0] - if 0 in x: + # convert from pyparsing.ParseResults, which doesn't support '0 in parse_result' + parse_result = parse_result.asList() + if len(parse_result) == 1: + return parse_result[0] + if 0 in parse_result: return float('nan') - x = [1. / e for e in x if isinstance(e, numbers.Number)] # Ignore || - return 1. / sum(x) + reciprocals = [1. / e for e in parse_result] + return 1. / sum(reciprocals) -def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 + +def sum_parse_action(parse_result): """ Add the inputs @@ -171,29 +173,35 @@ def sum_parse_action(x): # [ 1 + 2 - 3 ] -> 0 Allow a leading + or - """ total = 0.0 - op = ops['+'] - for e in x: - if e in set('+-'): - op = ops[e] + current_op = operator.add + for token in parse_result: + if token is '+': + current_op = operator.add + elif token is '-': + current_op = operator.sub else: - total = op(total, e) + total = current_op(total, token) return total -def prod_parse_action(x): # [ 1 * 2 / 3 ] => 0.66 + +def prod_parse_action(parse_result): """ Multiply the inputs [ 1, '*', 2, '/', 3 ] => 0.66 """ prod = 1.0 - op = ops['*'] - for e in x: - if e in set('*/'): - op = ops[e] + current_op = operator.mul + for token in parse_result: + if token is '*': + current_op = operator.mul + elif token is '/': + current_op = operator.truediv else: - prod = op(prod, e) + prod = current_op(prod, token) return prod + def evaluator(variables, functions, string, cs=False): """ Evaluate an expression. Variables are passed as a dictionary @@ -202,20 +210,12 @@ def evaluator(variables, functions, string, cs=False): cs: Case sensitive """ - # log.debug("variables: {0}".format(variables)) - # log.debug("functions: {0}".format(functions)) - # log.debug("string: {0}".format(string)) - - all_variables = copy.copy(default_variables) - all_functions = copy.copy(default_functions) - - def func_parse_action(x): - return [all_functions[x[0]](x[1])] - - if not cs: - all_variables = lower_dict(all_variables) - all_functions = lower_dict(all_functions) + # LOG.debug("variables: {0}".format(variables)) + # LOG.debug("functions: {0}".format(functions)) + # LOG.debug("string: {0}".format(string)) + all_variables = copy.copy(DEFAULT_VARIABLES) + all_functions = copy.copy(DEFAULT_FUNCTIONS) all_variables.update(variables) all_functions.update(functions) @@ -234,7 +234,7 @@ def evaluator(variables, functions, string, cs=False): return float('nan') # SI suffixes and percent - number_suffix = MatchFirst([Literal(k) for k in suffixes.keys()]) + number_suffix = MatchFirst([Literal(k) for k in SUFFIXES.keys()]) plus_minus = Literal('+') | Literal('-') times_div = Literal('*') | Literal('/') @@ -249,11 +249,10 @@ def evaluator(variables, functions, string, cs=False): number = (inner_number + Optional(CaselessLiteral("E") + Optional(plus_minus) + number_part) + Optional(number_suffix)) - number = number.setParseAction(number_parse_action) # Convert to number + number.setParseAction(number_parse_action) # Convert to number # Predefine recursive variables expr = Forward() - factor = Forward() # Handle variables passed in. E.g. if we have {'R':0.5}, we make the substitution. # Special case for no variables because of how we understand PyParsing is put together @@ -261,9 +260,10 @@ def evaluator(variables, functions, string, cs=False): # We sort the list so that var names (like "e2") match before # mathematical constants (like "e"). This is kind of a hack. all_variables_keys = sorted(all_variables.keys(), key=len, reverse=True) - literal_all_vars = [CasedLiteral(k) for k in all_variables_keys] - varnames = MatchFirst(literal_all_vars) - varnames.setParseAction(lambda x: [all_variables[k] for k in x]) + varnames = MatchFirst([CasedLiteral(k) for k in all_variables_keys]) + varnames.setParseAction( + lambda x: [all_variables[k] for k in x] + ) else: # all_variables includes DEFAULT_VARIABLES, which isn't empty # this is unreachable. Get rid of it? @@ -273,7 +273,9 @@ def evaluator(variables, functions, string, cs=False): if len(all_functions) > 0: funcnames = MatchFirst([CasedLiteral(k) for k in all_functions.keys()]) function = funcnames + Suppress("(") + expr + Suppress(")") - function.setParseAction(func_parse_action) + function.setParseAction( + lambda x: [all_functions[x[0]](x[1])] + ) else: # see note above (this is unreachable) function = NoMatch() @@ -281,11 +283,13 @@ def evaluator(variables, functions, string, cs=False): atom = number | function | varnames | Suppress("(") + expr + Suppress(")") # Do the following in the correct order to preserve order of operation - factor << (atom + ZeroOrMore("^" + atom)).setParseAction(exp_parse_action) # 7^6 - paritem = factor + ZeroOrMore(Literal('||') + factor) # 5k || 4k - paritem = paritem.setParseAction(parallel) - term = paritem + ZeroOrMore(times_div + paritem) # 7 * 5 / 4 - 3 - term = term.setParseAction(prod_parse_action) - expr << Optional(plus_minus) + term + ZeroOrMore(plus_minus + term) # -5 + 4 - 3 - expr = expr.setParseAction(sum_parse_action) + pow_term = atom + ZeroOrMore(Suppress("^") + atom) + pow_term.setParseAction(exp_parse_action) # 7^6 + par_term = pow_term + ZeroOrMore(Suppress('||') + pow_term) # 5k || 4k + par_term.setParseAction(parallel) + prod_term = par_term + ZeroOrMore(times_div + par_term) # 7 * 5 / 4 - 3 + prod_term.setParseAction(prod_parse_action) + sum_term = Optional(plus_minus) + prod_term + ZeroOrMore(plus_minus + prod_term) # -5 + 4 - 3 + sum_term.setParseAction(sum_parse_action) + expr << sum_term # finish the recursion return (expr + stringEnd).parseString(string)[0] From 83f1f9c2fc78442c77376457094ba674bca59c49 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 5 Jun 2013 15:50:35 -0400 Subject: [PATCH 05/55] Set numpy so it does not print out warnings on student input --- common/lib/calc/calc.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index f862b41542..cc3a883221 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -13,6 +13,10 @@ import re import numpy import scipy.constants +# have numpy raise errors on functions outside its domain +# See http://docs.scipy.org/doc/numpy/reference/generated/numpy.seterr.html +numpy.seterr(all='ignore') # Also: 'ignore', 'warn' (default), 'raise' + from pyparsing import (Word, nums, Literal, ZeroOrMore, MatchFirst, Optional, Forward, From b05ff885fad26d116b8755f7dd249038b6321389 Mon Sep 17 00:00:00 2001 From: Nate Hardison Date: Thu, 6 Jun 2013 14:18:08 -0700 Subject: [PATCH 06/55] Replace /faq route in LMS urls This route was mistakenly removed by the theming changes since the "FAQ" marketing link actually points to help_edx, not faq_edx. --- lms/urls.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/urls.py b/lms/urls.py index d2265463de..fc97c75a36 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -98,6 +98,8 @@ if not settings.MITX_FEATURES["USE_CUSTOM_THEME"]: url(r'^press$', 'student.views.press', name="press"), url(r'^media-kit$', 'static_template_view.views.render', {'template': 'media-kit.html'}, name="media-kit"), + url(r'^faq$', 'static_template_view.views.render', + {'template': 'faq.html'}, name="faq_edx"), url(r'^help$', 'static_template_view.views.render', {'template': 'help.html'}, name="help_edx"), @@ -125,7 +127,7 @@ for key, value in settings.MKTG_URL_LINK_MAP.items(): continue # These urls are enabled separately - if key == "ROOT" or key == "COURSES": + if key == "ROOT" or key == "COURSES" or key == "FAQ": continue # Make the assumptions that the templates are all in the same dir From 0baec0a164fda2881f043acf0d7f83dfe99e9a58 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 7 Jun 2013 15:45:34 -0400 Subject: [PATCH 07/55] Move string fields, get rid of hard-coded list of booleans. --- cms/xmodule_namespace.py | 2 - common/lib/xmodule/xmodule/capa_module.py | 16 +++--- .../xmodule/combined_open_ended_module.py | 20 +++---- common/lib/xmodule/xmodule/fields.py | 41 -------------- .../xmodule/xmodule/peer_grading_module.py | 23 ++++---- .../lib/xmodule/xmodule/tests/test_fields.py | 54 +------------------ .../xmodule/xmodule/tests/test_xml_module.py | 10 ++-- .../lib/xmodule/xmodule/word_cloud_module.py | 9 ++-- common/lib/xmodule/xmodule/xml_module.py | 35 +++++------- lms/xmodule_namespace.py | 8 +-- requirements/edx/github.txt | 2 +- 11 files changed, 56 insertions(+), 164 deletions(-) diff --git a/cms/xmodule_namespace.py b/cms/xmodule_namespace.py index 4857fe68ca..eef4b41f37 100644 --- a/cms/xmodule_namespace.py +++ b/cms/xmodule_namespace.py @@ -5,7 +5,6 @@ Namespace defining common fields used by Studio for all blocks import datetime from xblock.core import Namespace, Scope, ModelType, String -from xmodule.fields import StringyBoolean class DateTuple(ModelType): @@ -28,4 +27,3 @@ class CmsNamespace(Namespace): """ published_date = DateTuple(help="Date when the module was published", scope=Scope.settings) published_by = String(help="Id of the user who published this module", scope=Scope.settings) - diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 9ac540138e..38a0ea599a 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -18,8 +18,8 @@ from .progress import Progress from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from xblock.core import Scope, String, Boolean, Object -from .fields import Timedelta, Date, StringyInteger, StringyFloat +from xblock.core import Scope, String, Boolean, Object, Integer, Float +from .fields import Timedelta, Date from xmodule.util.date_utils import time_to_datetime log = logging.getLogger("mitx.courseware") @@ -65,8 +65,8 @@ class ComplexEncoder(json.JSONEncoder): class CapaFields(object): - attempts = StringyInteger(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state) - max_attempts = StringyInteger( + attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state) + max_attempts = Integer( display_name="Maximum Attempts", help="Defines the number of times a student can try to answer this problem. If the value is not set, infinite attempts are allowed.", values={"min": 1}, scope=Scope.settings @@ -99,8 +99,8 @@ class CapaFields(object): input_state = Object(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state) student_answers = Object(help="Dictionary with the current student responses", scope=Scope.user_state) done = Boolean(help="Whether the student has answered the problem", scope=Scope.user_state) - seed = StringyInteger(help="Random seed for this student", scope=Scope.user_state) - weight = StringyFloat( + seed = Integer(help="Random seed for this student", scope=Scope.user_state) + weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each response field in the problem is worth one point.", values={"min": 0, "step": .1}, @@ -315,7 +315,7 @@ class CapaModule(CapaFields, XModule): # If the user has forced the save button to display, # then show it as long as the problem is not closed # (past due / too many attempts) - if self.force_save_button == "true": + if self.force_save_button: return not self.closed() else: is_survey_question = (self.max_attempts == 0) @@ -782,7 +782,7 @@ class CapaModule(CapaFields, XModule): return {'success': msg} raise - self.attempts = self.attempts + 1 + self.attempts += 1 self.lcp.done = True self.set_state_from_lcp() diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index b3f0e19109..6c3725161a 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -5,10 +5,10 @@ from pkg_resources import resource_string from xmodule.raw_module import RawDescriptor from .x_module import XModule -from xblock.core import Integer, Scope, String, List +from xblock.core import Integer, Scope, String, List, Float, Boolean from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module, CombinedOpenEndedV1Descriptor from collections import namedtuple -from .fields import Date, StringyFloat, StringyInteger, StringyBoolean +from .fields import Date log = logging.getLogger("mitx.courseware") @@ -53,27 +53,27 @@ class CombinedOpenEndedFields(object): help="This name appears in the horizontal navigation at the top of the page.", default="Open Ended Grading", scope=Scope.settings ) - current_task_number = StringyInteger(help="Current task that the student is on.", default=0, scope=Scope.user_state) + current_task_number = Integer(help="Current task that the student is on.", default=0, scope=Scope.user_state) task_states = List(help="List of state dictionaries of each task within this module.", scope=Scope.user_state) state = String(help="Which step within the current task that the student is on.", default="initial", scope=Scope.user_state) - student_attempts = StringyInteger(help="Number of attempts taken by the student on this problem", default=0, + student_attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state) - ready_to_reset = StringyBoolean( + ready_to_reset = Boolean( help="If the problem is ready to be reset or not.", default=False, scope=Scope.user_state ) - attempts = StringyInteger( + attempts = Integer( display_name="Maximum Attempts", help="The number of times the student can try to answer this problem.", default=1, scope=Scope.settings, values = {"min" : 1 } ) - is_graded = StringyBoolean(display_name="Graded", help="Whether or not the problem is graded.", default=False, scope=Scope.settings) - accept_file_upload = StringyBoolean( + is_graded = Boolean(display_name="Graded", help="Whether or not the problem is graded.", default=False, scope=Scope.settings) + accept_file_upload = Boolean( display_name="Allow File Uploads", help="Whether or not the student can submit files as a response.", default=False, scope=Scope.settings ) - skip_spelling_checks = StringyBoolean( + skip_spelling_checks = Boolean( display_name="Disable Quality Filter", help="If False, the Quality Filter is enabled and submissions with poor spelling, short length, or poor grammar will not be peer reviewed.", default=False, scope=Scope.settings @@ -86,7 +86,7 @@ class CombinedOpenEndedFields(object): ) version = VersionInteger(help="Current version number", default=DEFAULT_VERSION, scope=Scope.settings) data = String(help="XML data for the problem", scope=Scope.content) - weight = StringyFloat( + weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", scope=Scope.settings, values = {"min" : 0 , "step": ".1"} diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 3d56b7941e..bb85714252 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -7,8 +7,6 @@ from xblock.core import ModelType import datetime import dateutil.parser -from xblock.core import Integer, Float, Boolean - log = logging.getLogger(__name__) @@ -83,42 +81,3 @@ class Timedelta(ModelType): if cur_value > 0: values.append("%d %s" % (cur_value, attr)) return ' '.join(values) - - -class StringyInteger(Integer): - """ - A model type that converts from strings to integers when reading from json. - If value does not parse as an int, returns None. - """ - def from_json(self, value): - try: - return int(value) - except: - return None - - -class StringyFloat(Float): - """ - A model type that converts from string to floats when reading from json. - If value does not parse as a float, returns None. - """ - def from_json(self, value): - try: - return float(value) - except: - return None - - -class StringyBoolean(Boolean): - """ - Reads strings from JSON as booleans. - - If the string is 'true' (case insensitive), then return True, - otherwise False. - - JSON values that aren't strings are returned as-is. - """ - def from_json(self, value): - if isinstance(value, basestring): - return value.lower() == 'true' - return value diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index ccc3e31f51..4dc553458b 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -10,8 +10,8 @@ from .x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.modulestore.django import modulestore from .timeinfo import TimeInfo -from xblock.core import Object, String, Scope -from xmodule.fields import Date, StringyFloat, StringyInteger, StringyBoolean +from xblock.core import Object, String, Scope, Boolean, Integer, Float +from xmodule.fields import Date from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService from open_ended_grading_classes import combined_open_ended_rubric @@ -20,7 +20,6 @@ log = logging.getLogger(__name__) USE_FOR_SINGLE_LOCATION = False LINK_TO_LOCATION = "" -TRUE_DICT = [True, "True", "true", "TRUE"] MAX_SCORE = 1 IS_GRADED = False @@ -28,7 +27,7 @@ EXTERNAL_GRADER_NO_CONTACT_ERROR = "Failed to contact external graders. Please class PeerGradingFields(object): - use_for_single_location = StringyBoolean( + use_for_single_location = Boolean( display_name="Show Single Problem", help='When True, only the single problem specified by "Link to Problem Location" is shown. ' 'When False, a panel is displayed with all problems available for peer grading.', @@ -39,14 +38,14 @@ class PeerGradingFields(object): help='The location of the problem being graded. Only used when "Show Single Problem" is True.', default=LINK_TO_LOCATION, scope=Scope.settings ) - is_graded = StringyBoolean( + is_graded = Boolean( display_name="Graded", help='Defines whether the student gets credit for grading this problem. Only used when "Show Single Problem" is True.', default=IS_GRADED, scope=Scope.settings ) due_date = Date(help="Due date that should be displayed.", default=None, scope=Scope.settings) grace_period_string = String(help="Amount of grace to give on the due date.", default=None, scope=Scope.settings) - max_grade = StringyInteger( + max_grade = Integer( help="The maximum grade that a student can receive for this problem.", default=MAX_SCORE, scope=Scope.settings, values={"min": 0} ) @@ -54,7 +53,7 @@ class PeerGradingFields(object): help="Student data for a given peer grading problem.", scope=Scope.user_state ) - weight = StringyFloat( + weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", scope=Scope.settings, values={"min": 0, "step": ".1"} @@ -84,7 +83,7 @@ class PeerGradingModule(PeerGradingFields, XModule): else: self.peer_gs = MockPeerGradingService() - if self.use_for_single_location in TRUE_DICT: + if self.use_for_single_location: try: self.linked_problem = modulestore().get_instance(self.system.course_id, self.link_to_location) except: @@ -146,7 +145,7 @@ class PeerGradingModule(PeerGradingFields, XModule): """ if self.closed(): return self.peer_grading_closed() - if self.use_for_single_location not in TRUE_DICT: + if not self.use_for_single_location: return self.peer_grading() else: return self.peer_grading_problem({'location': self.link_to_location})['html'] @@ -203,7 +202,7 @@ class PeerGradingModule(PeerGradingFields, XModule): 'score': score, 'total': max_score, } - if self.use_for_single_location not in TRUE_DICT or self.is_graded not in TRUE_DICT: + if not self.use_for_single_location or not self.is_graded: return score_dict try: @@ -238,7 +237,7 @@ class PeerGradingModule(PeerGradingFields, XModule): randomization, and 5/7 on another ''' max_grade = None - if self.use_for_single_location in TRUE_DICT and self.is_graded in TRUE_DICT: + if self.use_for_single_location and self.is_graded: max_grade = self.max_grade return max_grade @@ -556,7 +555,7 @@ class PeerGradingModule(PeerGradingFields, XModule): Show individual problem interface ''' if get is None or get.get('location') is None: - if self.use_for_single_location not in TRUE_DICT: + if not self.use_for_single_location: #This is an error case, because it must be set to use a single location to be called without get parameters #This is a dev_facing_error log.error( diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index 9642f7c595..e08508ac99 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -2,7 +2,7 @@ import datetime import unittest from django.utils.timezone import UTC -from xmodule.fields import Date, StringyFloat, StringyInteger, StringyBoolean +from xmodule.fields import Date import time class DateTest(unittest.TestCase): @@ -78,55 +78,3 @@ class DateTest(unittest.TestCase): DateTest.date.from_json("2012-12-31T23:00:01-01:00")), "2013-01-01T00:00:01Z") - -class StringyIntegerTest(unittest.TestCase): - def assertEquals(self, expected, arg): - self.assertEqual(expected, StringyInteger().from_json(arg)) - - def test_integer(self): - self.assertEquals(5, '5') - self.assertEquals(0, '0') - self.assertEquals(-1023, '-1023') - - def test_none(self): - self.assertEquals(None, None) - self.assertEquals(None, 'abc') - self.assertEquals(None, '[1]') - self.assertEquals(None, '1.023') - - -class StringyFloatTest(unittest.TestCase): - - def assertEquals(self, expected, arg): - self.assertEqual(expected, StringyFloat().from_json(arg)) - - def test_float(self): - self.assertEquals(.23, '.23') - self.assertEquals(5, '5') - self.assertEquals(0, '0.0') - self.assertEquals(-1023.22, '-1023.22') - - def test_none(self): - self.assertEquals(None, None) - self.assertEquals(None, 'abc') - self.assertEquals(None, '[1]') - - -class StringyBooleanTest(unittest.TestCase): - - def assertEquals(self, expected, arg): - self.assertEqual(expected, StringyBoolean().from_json(arg)) - - def test_false(self): - self.assertEquals(False, "false") - self.assertEquals(False, "False") - self.assertEquals(False, "") - self.assertEquals(False, "hahahahah") - - def test_true(self): - self.assertEquals(True, "true") - self.assertEquals(True, "TruE") - - def test_pass_through(self): - self.assertEquals(123, 123) - diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index dd59ca2b48..18cd11650f 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -2,8 +2,8 @@ #pylint: disable=C0111 from xmodule.x_module import XModuleFields -from xblock.core import Scope, String, Object, Boolean -from xmodule.fields import Date, StringyInteger, StringyFloat +from xblock.core import Scope, String, Object, Boolean, Integer, Float +from xmodule.fields import Date from xmodule.xml_module import XmlDescriptor import unittest from .import test_system @@ -17,7 +17,7 @@ class CrazyJsonString(String): class TestFields(object): # Will be returned by editable_metadata_fields. - max_attempts = StringyInteger(scope=Scope.settings, default=1000, values={'min': 1, 'max': 10}) + max_attempts = Integer(scope=Scope.settings, default=1000, values={'min': 1, 'max': 10}) # Will not be returned by editable_metadata_fields because filtered out by non_editable_metadata_fields. due = Date(scope=Scope.settings) # Will not be returned by editable_metadata_fields because is not Scope.settings. @@ -33,9 +33,9 @@ class TestFields(object): {'display_name': 'second', 'value': 'value b'}] ) # Used for testing select type - float_select = StringyFloat(scope=Scope.settings, default=.999, values=[1.23, 0.98]) + float_select = Float(scope=Scope.settings, default=.999, values=[1.23, 0.98]) # Used for testing float type - float_non_select = StringyFloat(scope=Scope.settings, default=.999, values={'min': 0, 'step': .3}) + float_non_select = Float(scope=Scope.settings, default=.999, values={'min': 0, 'step': .3}) # Used for testing that Booleans get mapped to select type boolean_select = Boolean(scope=Scope.settings) diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index e38b8cf195..6605f9b870 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -14,8 +14,7 @@ from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule -from xblock.core import Scope, Object, Boolean, List -from fields import StringyBoolean, StringyInteger +from xblock.core import Scope, Object, Boolean, List, Integer log = logging.getLogger(__name__) @@ -32,21 +31,21 @@ def pretty_bool(value): class WordCloudFields(object): """XFields for word cloud.""" - num_inputs = StringyInteger( + num_inputs = Integer( display_name="Inputs", help="Number of text boxes available for students to input words/sentences.", scope=Scope.settings, default=5, values={"min": 1} ) - num_top_words = StringyInteger( + num_top_words = Integer( display_name="Maximum Words", help="Maximum number of words to be displayed in generated word cloud.", scope=Scope.settings, default=250, values={"min": 1} ) - display_student_percents = StringyBoolean( + display_student_percents = Boolean( display_name="Show Percents", help="Statistics are shown for entered words near that word.", scope=Scope.settings, diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 2f54bbf405..56f8b6fd15 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -120,25 +120,15 @@ class XmlDescriptor(XModuleDescriptor): metadata_to_export_to_policy = ('discussion_topics') - # A dictionary mapping xml attribute names AttrMaps that describe how - # to import and export them - # Allow json to specify either the string "true", or the bool True. The string is preferred. - to_bool = lambda val: val == 'true' or val == True - from_bool = lambda val: str(val).lower() - bool_map = AttrMap(to_bool, from_bool) - - to_int = lambda val: int(val) - from_int = lambda val: str(val) - int_map = AttrMap(to_int, from_int) - xml_attribute_map = { - # type conversion: want True/False in python, "true"/"false" in xml - 'graded': bool_map, - 'hide_progress_tab': bool_map, - 'allow_anonymous': bool_map, - 'allow_anonymous_to_peers': bool_map, - 'show_timezone': bool_map, - } + @classmethod + def get_map_for_field(cls, attr): + for field in set(cls.fields + cls.lms.fields): + if field.name == attr: + from_xml = lambda val: field.deserialize(val) + to_xml = lambda val : field.serialize(val) + return AttrMap(from_xml, to_xml) + return AttrMap() @classmethod def definition_from_xml(cls, xml_object, system): @@ -188,7 +178,6 @@ class XmlDescriptor(XModuleDescriptor): filepath, location.url(), str(err)) raise Exception, msg, sys.exc_info()[2] - @classmethod def load_definition(cls, xml_object, system, location): '''Load a descriptor definition from the specified xml_object. @@ -246,7 +235,7 @@ class XmlDescriptor(XModuleDescriptor): # don't load these continue - attr_map = cls.xml_attribute_map.get(attr, AttrMap()) + attr_map = cls.get_map_for_field(attr) metadata[attr] = attr_map.from_xml(val) return metadata @@ -258,7 +247,7 @@ class XmlDescriptor(XModuleDescriptor): through the attrmap. Updates the metadata dict in place. """ for attr in policy: - attr_map = cls.xml_attribute_map.get(attr, AttrMap()) + attr_map = cls.get_map_for_field(attr) metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr]) @classmethod @@ -347,7 +336,7 @@ class XmlDescriptor(XModuleDescriptor): def export_to_xml(self, resource_fs): """ - Returns an xml string representign this module, and all modules + Returns an xml string representing this module, and all modules underneath it. May also write required resources out to resource_fs Assumes that modules have single parentage (that no module appears twice @@ -372,7 +361,7 @@ class XmlDescriptor(XModuleDescriptor): """Get the value for this attribute that we want to store. (Possible format conversion through an AttrMap). """ - attr_map = self.xml_attribute_map.get(attr, AttrMap()) + attr_map = self.get_map_for_field(attr) return attr_map.to_xml(self._model_data[attr]) # Add the non-inherited metadata diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py index 6b78d18db0..aaef0b76db 100644 --- a/lms/xmodule_namespace.py +++ b/lms/xmodule_namespace.py @@ -1,15 +1,15 @@ """ Namespace that defines fields common to all blocks used in the LMS """ -from xblock.core import Namespace, Boolean, Scope, String -from xmodule.fields import Date, Timedelta, StringyFloat, StringyBoolean +from xblock.core import Namespace, Boolean, Scope, String, Float +from xmodule.fields import Date, Timedelta class LmsNamespace(Namespace): """ Namespace that defines fields common to all blocks used in the LMS """ - hide_from_toc = StringyBoolean( + hide_from_toc = Boolean( help="Whether to display this module in the table of contents", default=False, scope=Scope.settings @@ -37,7 +37,7 @@ class LmsNamespace(Namespace): ) showanswer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed") rerandomize = String(help="When to rerandomize the problem", default="always", scope=Scope.settings) - days_early_for_beta = StringyFloat( + days_early_for_beta = Float( help="Number of days early to show content to beta users", default=None, scope=Scope.settings diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index fc9070bba3..fb20fd2b22 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,6 +8,6 @@ -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@2144a25d#egg=XBlock +-e git+https://github.com/edx/XBlock.git@a56a79d8#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From f3b92312d920a60c3d74b91b20355a3c8b3dd11d Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 7 Jun 2013 15:58:31 -0400 Subject: [PATCH 08/55] Add test for serialize/deserialize. --- common/lib/xmodule/xmodule/tests/test_fields.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index e08508ac99..a5730d55b3 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -78,3 +78,18 @@ class DateTest(unittest.TestCase): DateTest.date.from_json("2012-12-31T23:00:01-01:00")), "2013-01-01T00:00:01Z") + def test_serialize(self): + self.assertEqual( + DateTest.date.serialize("2012-12-31T23:59:59Z"), + '"2012-12-31T23:59:59Z"' + ) + + def test_deserialize(self): + self.assertEqual( + '2012-12-31T23:59:59Z', + DateTest.date.deserialize("2012-12-31T23:59:59Z"), + ) + self.assertEqual( + '2012-12-31T23:59:59Z', + DateTest.date.deserialize('"2012-12-31T23:59:59Z"'), + ) From 1273bc22b389b657dbc7b8e2fdd4278af6946491 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 7 Jun 2013 17:24:07 -0400 Subject: [PATCH 09/55] Additional test coverage. --- common/lib/xmodule/xmodule/capa_module.py | 2 +- .../lib/xmodule/xmodule/tests/test_fields.py | 39 ++++++++++++++++++- common/test/data/full/course.xml | 2 +- requirements/edx/github.txt | 2 +- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 38a0ea599a..392d29134b 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -782,7 +782,7 @@ class CapaModule(CapaFields, XModule): return {'success': msg} raise - self.attempts += 1 + self.attempts = self.attempts + 1 self.lcp.done = True self.set_state_from_lcp() diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index a5730d55b3..d944566e97 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -2,7 +2,7 @@ import datetime import unittest from django.utils.timezone import UTC -from xmodule.fields import Date +from xmodule.fields import Date, Timedelta import time class DateTest(unittest.TestCase): @@ -93,3 +93,40 @@ class DateTest(unittest.TestCase): '2012-12-31T23:59:59Z', DateTest.date.deserialize('"2012-12-31T23:59:59Z"'), ) + + +class TimedeltaTest(unittest.TestCase): + delta = Timedelta() + + def test_from_json(self): + self.assertEqual( + TimedeltaTest.delta.from_json('1 day 12 hours 59 minutes 59 seconds'), + datetime.timedelta(days=1, hours=12, minutes=59, seconds=59) + ) + + self.assertEqual( + TimedeltaTest.delta.from_json('1 day 46799 seconds'), + datetime.timedelta(days=1, seconds=46799) + ) + + def test_to_json(self): + self.assertEqual( + '1 days 46799 seconds', + TimedeltaTest.delta.to_json(datetime.timedelta(days=1, hours=12, minutes=59, seconds=59)) + ) + + def test_serialize(self): + self.assertEqual( + TimedeltaTest.delta.serialize('1 day 12 hours 59 minutes 59 seconds'), + '"1 day 12 hours 59 minutes 59 seconds"' + ) + + def test_deserialize(self): + self.assertEqual( + '1 day 12 hours 59 minutes 59 seconds', + TimedeltaTest.delta.deserialize('1 day 12 hours 59 minutes 59 seconds') + ) + self.assertEqual( + '1 day 12 hours 59 minutes 59 seconds', + TimedeltaTest.delta.deserialize('"1 day 12 hours 59 minutes 59 seconds"') + ) diff --git a/common/test/data/full/course.xml b/common/test/data/full/course.xml index b2f9097020..9ee128da1a 100644 --- a/common/test/data/full/course.xml +++ b/common/test/data/full/course.xml @@ -1 +1 @@ - + diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index fb20fd2b22..668ac4804c 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,6 +8,6 @@ -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@a56a79d8#egg=XBlock +-e git+https://github.com/edx/XBlock.git@eaaf4831#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From a746a9ad1511e5d02cea41a63250d3409d7868a9 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Mon, 10 Jun 2013 11:02:19 -0400 Subject: [PATCH 10/55] Get rid of unused code --- common/lib/calc/calc.py | 44 +++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index cc3a883221..349810d4c9 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -21,7 +21,7 @@ from pyparsing import (Word, nums, Literal, ZeroOrMore, MatchFirst, Optional, Forward, CaselessLiteral, - NoMatch, stringEnd, Suppress, Combine) + stringEnd, Suppress, Combine) DEFAULT_FUNCTIONS = {'sin': numpy.sin, 'cos': numpy.cos, @@ -258,31 +258,27 @@ def evaluator(variables, functions, string, cs=False): # Predefine recursive variables expr = Forward() - # Handle variables passed in. E.g. if we have {'R':0.5}, we make the substitution. - # Special case for no variables because of how we understand PyParsing is put together - if len(all_variables) > 0: - # We sort the list so that var names (like "e2") match before - # mathematical constants (like "e"). This is kind of a hack. - all_variables_keys = sorted(all_variables.keys(), key=len, reverse=True) - varnames = MatchFirst([CasedLiteral(k) for k in all_variables_keys]) - varnames.setParseAction( - lambda x: [all_variables[k] for k in x] - ) - else: - # all_variables includes DEFAULT_VARIABLES, which isn't empty - # this is unreachable. Get rid of it? - varnames = NoMatch() + # Handle variables passed in. + # E.g. if we have {'R':0.5}, we make the substitution. + # We sort the list so that var names (like "e2") match before + # mathematical constants (like "e"). This is kind of a hack. + all_variables_keys = sorted(all_variables.keys(), key=len, reverse=True) + varnames = MatchFirst([CasedLiteral(k) for k in all_variables_keys]) + varnames.setParseAction( + lambda x: [all_variables[k] for k in x] + ) + + # if all_variables were empty, then pyparsing wants + # varnames = NoMatch() + # this is not the case, as all_variables contains the defaults # Same thing for functions. - if len(all_functions) > 0: - funcnames = MatchFirst([CasedLiteral(k) for k in all_functions.keys()]) - function = funcnames + Suppress("(") + expr + Suppress(")") - function.setParseAction( - lambda x: [all_functions[x[0]](x[1])] - ) - else: - # see note above (this is unreachable) - function = NoMatch() + all_functions_keys = sorted(all_functions.keys(), key=len, reverse=True) + funcnames = MatchFirst([CasedLiteral(k) for k in all_functions_keys]) + function = funcnames + Suppress("(") + expr + Suppress(")") + function.setParseAction( + lambda x: [all_functions[x[0]](x[1])] + ) atom = number | function | varnames | Suppress("(") + expr + Suppress(")") From 58e98d13cc5df9f0ff2994719fcf152219ada507 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Mon, 10 Jun 2013 11:34:59 -0400 Subject: [PATCH 11/55] Make Jenkins test the calc module --- jenkins/test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/jenkins/test.sh b/jenkins/test.sh index 35be3a0121..127bf4fa1d 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -77,6 +77,7 @@ rake test_cms || TESTS_FAILED=1 rake test_lms || TESTS_FAILED=1 rake test_common/lib/capa || TESTS_FAILED=1 rake test_common/lib/xmodule || TESTS_FAILED=1 +rake test_common/lib/calc || TESTS_FAILED=1 # Run the javascript unit tests rake phantomjs_jasmine_lms || TESTS_FAILED=1 From ed90ed9a345aac35cbdc878ddc484475923c08fd Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Wed, 5 Jun 2013 15:36:49 -0400 Subject: [PATCH 12/55] Added tests for new math functions --- common/lib/calc/tests/test_calc.py | 99 ++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/common/lib/calc/tests/test_calc.py b/common/lib/calc/tests/test_calc.py index cfa1b7525d..e29c6776a9 100644 --- a/common/lib/calc/tests/test_calc.py +++ b/common/lib/calc/tests/test_calc.py @@ -194,6 +194,105 @@ class EvaluatorTest(unittest.TestCase): arctan_angles = arcsin_angles self.assert_function_values('arctan', arctan_inputs, arctan_angles) + def test_reciprocal_trig_functions(self): + """ + Test the reciprocal trig functions provided in calc.py + + which are: sec, csc, cot, arcsec, arccsc, arccot + """ + angles = ['-pi/4', 'pi/6', 'pi/5', '5*pi/4', '9*pi/4', '1 + j'] + sec_values = [1.414, 1.155, 1.236, -1.414, 1.414, 0.498+0.591j] + csc_values = [-1.414, 2, 1.701, -1.414, 1.414, 0.622-0.304j] + cot_values = [-1, 1.732, 1.376, 1, 1, 0.218-0.868j] + + self.assert_function_values('sec', angles, sec_values) + self.assert_function_values('csc', angles, csc_values) + self.assert_function_values('cot', angles, cot_values) + + arcsec_inputs = ['1.1547', '1.2361', '2', '-2', '-1.4142', '0.4983+0.5911*j'] + arcsec_angles = [0.524, 0.628, 1.047, 2.094, 2.356, 1 + 1j] + self.assert_function_values('arcsec', arcsec_inputs, arcsec_angles) + + arccsc_inputs = ['-1.1547', '-1.4142', '2', '1.7013', '1.1547', '0.6215-0.3039*j'] + arccsc_angles = [-1.047, -0.785, 0.524, 0.628, 1.047, 1 + 1j] + self.assert_function_values('arccsc', arccsc_inputs, arccsc_angles) + + # Has the same range as arccsc + arccot_inputs = ['-0.5774', '-1', '1.7321', '1.3764', '0.5774', '(0.2176-0.868*j)'] + arccot_angles = arccsc_angles + self.assert_function_values('arccot', arccot_inputs, arccot_angles) + + def test_hyperbolic_functions(self): + """ + Test the hyperbolic functions + + which are: sinh, cosh, tanh, sech, csch, coth + """ + inputs = ['0', '0.5', '1', '2', '1+j'] + neg_inputs = ['0', '-0.5', '-1', '-2', '-1-j'] + negate = lambda x: [-k for k in x] + + # sinh is odd + sinh_vals = [0, 0.521, 1.175, 3.627, 0.635 + 1.298j] + self.assert_function_values('sinh', inputs, sinh_vals) + self.assert_function_values('sinh', neg_inputs, negate(sinh_vals)) + + # cosh is even - do not negate + cosh_vals = [1, 1.128, 1.543, 3.762, 0.834 + 0.989j] + self.assert_function_values('cosh', inputs, cosh_vals) + self.assert_function_values('cosh', neg_inputs, cosh_vals) + + # tanh is odd + tanh_vals = [0, 0.462, 0.762, 0.964, 1.084 + 0.272j] + self.assert_function_values('tanh', inputs, tanh_vals) + self.assert_function_values('tanh', neg_inputs, negate(tanh_vals)) + + # sech is even - do not negate + sech_vals = [1, 0.887, 0.648, 0.266, 0.498 - 0.591j] + self.assert_function_values('sech', inputs, sech_vals) + self.assert_function_values('sech', neg_inputs, sech_vals) + + # the following functions do not have 0 in their domain + inputs = inputs[1:] + neg_inputs = neg_inputs[1:] + + # csch is odd + csch_vals = [1.919, 0.851, 0.276, 0.304 - 0.622j] + self.assert_function_values('csch', inputs, csch_vals) + self.assert_function_values('csch', neg_inputs, negate(csch_vals)) + + # coth is odd + coth_vals = [2.164, 1.313, 1.037, 0.868 - 0.218j] + self.assert_function_values('coth', inputs, coth_vals) + self.assert_function_values('coth', neg_inputs, negate(coth_vals)) + + def test_hyperbolic_inverses(self): + """ + Test the inverse hyperbolic functions + + which are of the form arc[X]h + """ + results = [0, 0.5, 1, 2, 1+1j] + + sinh_vals = ['0', '0.5211', '1.1752', '3.6269', '0.635+1.2985*j'] + self.assert_function_values('arcsinh', sinh_vals, results) + + cosh_vals = ['1', '1.1276', '1.5431', '3.7622', '0.8337+0.9889*j'] + self.assert_function_values('arccosh', cosh_vals, results) + + tanh_vals = ['0', '0.4621', '0.7616', '0.964', '1.0839+0.2718*j'] + self.assert_function_values('arctanh', tanh_vals, results) + + sech_vals = ['1.0', '0.8868', '0.6481', '0.2658', '0.4983-0.5911*j'] + self.assert_function_values('arcsech', sech_vals, results) + + results = results[1:] + csch_vals = ['1.919', '0.8509', '0.2757', '0.3039-0.6215*j'] + self.assert_function_values('arccsch', csch_vals, results) + + coth_vals = ['2.164', '1.313', '1.0373', '0.868-0.2176*j'] + self.assert_function_values('arccoth', coth_vals, results) + def test_other_functions(self): """ Test the non-trig functions provided in calc.py From 944e3390e0f4f63b90e87b65e529115c8d8b26e0 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Mon, 3 Jun 2013 17:20:52 -0400 Subject: [PATCH 13/55] Add support for various math functions in calc.py. --- common/lib/calc/calc.py | 22 ++++++- common/lib/calc/calcfunctions.py | 99 ++++++++++++++++++++++++++++++ common/lib/calc/tests/test_calc.py | 8 +-- 3 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 common/lib/calc/calcfunctions.py diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 349810d4c9..d3874639bc 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -12,6 +12,7 @@ import re import numpy import scipy.constants +import calcfunctions # have numpy raise errors on functions outside its domain # See http://docs.scipy.org/doc/numpy/reference/generated/numpy.seterr.html @@ -26,16 +27,35 @@ from pyparsing import (Word, nums, Literal, DEFAULT_FUNCTIONS = {'sin': numpy.sin, 'cos': numpy.cos, 'tan': numpy.tan, + 'sec': calcfunctions.sec, + 'csc': calcfunctions.csc, + 'cot': calcfunctions.cot, 'sqrt': numpy.sqrt, 'log10': numpy.log10, 'log2': numpy.log2, 'ln': numpy.log, + 'exp': numpy.exp, 'arccos': numpy.arccos, 'arcsin': numpy.arcsin, 'arctan': numpy.arctan, + 'arcsec': calcfunctions.arcsec, + 'arccsc': calcfunctions.arccsc, + 'arccot': calcfunctions.arccot, 'abs': numpy.abs, 'fact': math.factorial, - 'factorial': math.factorial + 'factorial': math.factorial, + 'sinh': numpy.sinh, + 'cosh': numpy.cosh, + 'tanh': numpy.tanh, + 'sech': calcfunctions.sech, + 'csch': calcfunctions.csch, + 'coth': calcfunctions.coth, + 'arcsinh': numpy.arcsinh, + 'arccosh': numpy.arccosh, + 'arctanh': numpy.arctanh, + 'arcsech': calcfunctions.arcsech, + 'arccsch': calcfunctions.arccsch, + 'arccoth': calcfunctions.arccoth } DEFAULT_VARIABLES = {'j': numpy.complex(0, 1), 'e': numpy.e, diff --git a/common/lib/calc/calcfunctions.py b/common/lib/calc/calcfunctions.py new file mode 100644 index 0000000000..d0ac4f7a3d --- /dev/null +++ b/common/lib/calc/calcfunctions.py @@ -0,0 +1,99 @@ +""" +Provide the mathematical functions that numpy doesn't. + +Specifically, the secant/cosecant/cotangents and their inverses and +hyperbolic counterparts +""" +import numpy + + +# Normal Trig +def sec(arg): + """ + Secant + """ + return 1 / numpy.cos(arg) + + +def csc(arg): + """ + Cosecant + """ + return 1 / numpy.sin(arg) + + +def cot(arg): + """ + Cotangent + """ + return 1 / numpy.tan(arg) + + +# Inverse Trig +# http://en.wikipedia.org/wiki/Inverse_trigonometric_functions#Relationships_among_the_inverse_trigonometric_functions +def arcsec(val): + """ + Inverse secant + """ + return numpy.arccos(1. / val) + + +def arccsc(val): + """ + Inverse cosecant + """ + return numpy.arcsin(1. / val) + + +def arccot(val): + """ + Inverse cotangent + """ + if numpy.real(val) < 0: + return -numpy.pi / 2 - numpy.arctan(val) + else: + return numpy.pi / 2 - numpy.arctan(val) + + +# Hyperbolic Trig +def sech(arg): + """ + Hyperbolic secant + """ + return 1 / numpy.cosh(arg) + + +def csch(arg): + """ + Hyperbolic cosecant + """ + return 1 / numpy.sinh(arg) + + +def coth(arg): + """ + Hyperbolic cotangent + """ + return 1 / numpy.tanh(arg) + + +# And their inverses +def arcsech(val): + """ + Inverse hyperbolic secant + """ + return numpy.arccosh(1. / val) + + +def arccsch(val): + """ + Inverse hyperbolic cosecant + """ + return numpy.arcsinh(1. / val) + + +def arccoth(val): + """ + Inverse hyperbolic cotangent + """ + return numpy.arctanh(1. / val) diff --git a/common/lib/calc/tests/test_calc.py b/common/lib/calc/tests/test_calc.py index e29c6776a9..13cd9e9471 100644 --- a/common/lib/calc/tests/test_calc.py +++ b/common/lib/calc/tests/test_calc.py @@ -201,9 +201,9 @@ class EvaluatorTest(unittest.TestCase): which are: sec, csc, cot, arcsec, arccsc, arccot """ angles = ['-pi/4', 'pi/6', 'pi/5', '5*pi/4', '9*pi/4', '1 + j'] - sec_values = [1.414, 1.155, 1.236, -1.414, 1.414, 0.498+0.591j] - csc_values = [-1.414, 2, 1.701, -1.414, 1.414, 0.622-0.304j] - cot_values = [-1, 1.732, 1.376, 1, 1, 0.218-0.868j] + sec_values = [1.414, 1.155, 1.236, -1.414, 1.414, 0.498 + 0.591j] + csc_values = [-1.414, 2, 1.701, -1.414, 1.414, 0.622 - 0.304j] + cot_values = [-1, 1.732, 1.376, 1, 1, 0.218 - 0.868j] self.assert_function_values('sec', angles, sec_values) self.assert_function_values('csc', angles, csc_values) @@ -272,7 +272,7 @@ class EvaluatorTest(unittest.TestCase): which are of the form arc[X]h """ - results = [0, 0.5, 1, 2, 1+1j] + results = [0, 0.5, 1, 2, 1 + 1j] sinh_vals = ['0', '0.5211', '1.1752', '3.6269', '0.635+1.2985*j'] self.assert_function_values('arcsinh', sinh_vals, results) From 0f72eedd37285e59a2e93932e4a2c1c967053376 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Mon, 10 Jun 2013 10:51:17 -0400 Subject: [PATCH 14/55] Add variable i as an imaginary unit --- common/lib/calc/calc.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index d3874639bc..3afc0f91bc 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -57,7 +57,8 @@ DEFAULT_FUNCTIONS = {'sin': numpy.sin, 'arccsch': calcfunctions.arccsch, 'arccoth': calcfunctions.arccoth } -DEFAULT_VARIABLES = {'j': numpy.complex(0, 1), +DEFAULT_VARIABLES = {'i': numpy.complex(0, 1), + 'j': numpy.complex(0, 1), 'e': numpy.e, 'pi': numpy.pi, 'k': scipy.constants.k, From 17c9c104d90680f270529d725a27504651535e80 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 10 Jun 2013 12:29:49 -0400 Subject: [PATCH 15/55] Update version of xblock. --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 668ac4804c..36fd9dca06 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,6 +8,6 @@ -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@eaaf4831#egg=XBlock +-e git+https://github.com/edx/XBlock.git@4c5d2397#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From 8f49783da0e8eeb5d6e55948c5804cdafff26e01 Mon Sep 17 00:00:00 2001 From: John Kern Date: Mon, 10 Jun 2013 12:58:57 -0700 Subject: [PATCH 16/55] encoded URL to fix formating --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3a6236ea70..92a4116354 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ CMS templates. Fortunately, `rake` will do all of this for you! Just run: If you are running these commands using the [`zsh`](http://www.zsh.org/) shell, zsh will assume that you are doing -[shell globbing](https://en.wikipedia.org/wiki/Glob_(programming)), search for +[shell globbing](https://en.wikipedia.org/wiki/Glob_%28programming%29), search for a file in your directory named `django-adminsyncdb` or `django-adminmigrate`, and fail. To fix this, just surround the argument with quotation marks, so that you're running `rake "django-admin[syncdb]"`. From 7b7a3427d91cd09d2e40e7e463caf52daf193cc3 Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Tue, 11 Jun 2013 15:16:12 -0400 Subject: [PATCH 17/55] Fix broken lettuce tests --- lms/djangoapps/courseware/features/problems.feature | 6 +++--- lms/djangoapps/courseware/features/problems.py | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature index 9a2e908438..4a5e64e9f4 100644 --- a/lms/djangoapps/courseware/features/problems.feature +++ b/lms/djangoapps/courseware/features/problems.feature @@ -113,9 +113,9 @@ Feature: Answer problems Scenario: I can view and hide the answer if the problem has it: Given I am viewing a "numerical" that shows the answer "always" When I press the button with the label "Show Answer(s)" - Then The "Hide Answer(s)" button does appear - And The "Show Answer(s)" button does not appear + Then the button with the label "Hide Answer(s)" does appear + And the button with the label "Show Answer(s)" does not appear And I should see "4.14159" somewhere in the page When I press the button with the label "Hide Answer(s)" - Then The "Show Answer(s)" button does appear + Then the button with the label "Show Answer(s)" does appear And I should not see "4.14159" anywhere on the page diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index 0ad005d70a..4245e7ca86 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -126,6 +126,15 @@ def press_the_button_with_label(step, buttonname): @step(u'The "([^"]*)" button does( not)? appear') def action_button_present(step, buttonname, doesnt_appear): + button_css = 'section.action input[value*="%s"]' % buttonname + if doesnt_appear: + assert world.is_css_not_present(button_css) + else: + assert world.is_css_present(button_css) + + +@step(u'the button with the label "([^"]*)" does( not)? appear') +def button_with_label_present(step, buttonname, doesnt_appear): button_css = 'button span.show-label' elem = world.css_find(button_css).first if doesnt_appear: From 20df6dec00b7dc65371ee4ac2cccef1ecc95607a Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 12 Jun 2013 10:57:31 -0400 Subject: [PATCH 18/55] Default to unicode for sass compilation. If no default is provided, sass falls back to the system default. This fails when the system default is not unicode because various sass files have unicode characters in them. --- rakefiles/assets.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rakefiles/assets.rake b/rakefiles/assets.rake index 009c87048c..4530ffe13b 100644 --- a/rakefiles/assets.rake +++ b/rakefiles/assets.rake @@ -52,7 +52,7 @@ def sass_cmd(watch=false, debug=false) "sass #{debug ? '--debug-info' : '--style compressed'} " + "--load-path #{sass_load_paths.join(' ')} " + "--require ./common/static/sass/bourbon/lib/bourbon.rb " + - "#{watch ? '--watch' : '--update'} #{sass_watch_paths.join(' ')}" + "#{watch ? '--watch' : '--update'} -E utf-8 #{sass_watch_paths.join(' ')}" end desc "Compile all assets" From d380c76df628258f91a9f2db6acce1f87397f9cf Mon Sep 17 00:00:00 2001 From: e0d Date: Tue, 11 Jun 2013 14:42:39 -0400 Subject: [PATCH 19/55] Adding a new env to handle escalating privileges to run db migrations. (cherry picked from commit d2554909d8d67a58c962a693ef3b3e5a0d3f3ee8) --- lms/envs/aws_migrate.py | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 lms/envs/aws_migrate.py diff --git a/lms/envs/aws_migrate.py b/lms/envs/aws_migrate.py new file mode 100644 index 0000000000..6ba81f04e0 --- /dev/null +++ b/lms/envs/aws_migrate.py @@ -0,0 +1,8 @@ +from .aws import * +import os + +USER = os.environ.get('DB_MIGRATION_USER', 'root') +PASSWORD = os.environ.get('DB_MIGRATION_PASS', None) + +DATABASES['default']['USER'] = USER +DATABASES['default']['PASSWORD'] = PASSWORD From 3ae11e860ca900ecbda19c6c5eb36e832e9ef2f9 Mon Sep 17 00:00:00 2001 From: e0d Date: Tue, 11 Jun 2013 15:00:39 -0400 Subject: [PATCH 20/55] Better error handling. (cherry picked from commit 1511ec841a30a7a8adbec2d700502a1e685f6103) --- lms/envs/aws_migrate.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lms/envs/aws_migrate.py b/lms/envs/aws_migrate.py index 6ba81f04e0..fae928868e 100644 --- a/lms/envs/aws_migrate.py +++ b/lms/envs/aws_migrate.py @@ -4,5 +4,9 @@ import os USER = os.environ.get('DB_MIGRATION_USER', 'root') PASSWORD = os.environ.get('DB_MIGRATION_PASS', None) +if not PASSWORD: + raise ImproperlyConfigured("No database password was provided for running " + "migrations. This is fatal.") + DATABASES['default']['USER'] = USER DATABASES['default']['PASSWORD'] = PASSWORD From bcc92ef8ef93eb65394cccba3fa3f6e251af08ff Mon Sep 17 00:00:00 2001 From: e0d Date: Tue, 11 Jun 2013 16:37:37 -0400 Subject: [PATCH 21/55] Adding comments. (cherry picked from commit c96511a278ad86cbcc5353ea7e3ea74519b73634) --- lms/envs/aws_migrate.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lms/envs/aws_migrate.py b/lms/envs/aws_migrate.py index fae928868e..595da6d78f 100644 --- a/lms/envs/aws_migrate.py +++ b/lms/envs/aws_migrate.py @@ -1,3 +1,10 @@ +""" +A Django settings file for use on AWS while running +database migrations, since we don't want to normally run the +LMS with enough privileges to modify the database schema. +""" + +# Import everything from .aws so that our settings are based on those. from .aws import * import os From 9acab2de703a437f8d5db19616df5324e8a9e2b6 Mon Sep 17 00:00:00 2001 From: e0d Date: Wed, 12 Jun 2013 11:24:28 -0400 Subject: [PATCH 22/55] added pylint ignores, fixed missing import (cherry picked from commit 273df4e11399d01c5dd5bb9ea1fcef9adf0414e5) --- lms/envs/aws_migrate.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lms/envs/aws_migrate.py b/lms/envs/aws_migrate.py index 595da6d78f..726b09874c 100644 --- a/lms/envs/aws_migrate.py +++ b/lms/envs/aws_migrate.py @@ -4,9 +4,14 @@ database migrations, since we don't want to normally run the LMS with enough privileges to modify the database schema. """ +# We intentionally define lots of variables that aren't used, and +# want to import all variables from base settings files +# pylint: disable=W0401, W0614 + # Import everything from .aws so that our settings are based on those. from .aws import * import os +from django.core.exceptions import ImproperlyConfigured USER = os.environ.get('DB_MIGRATION_USER', 'root') PASSWORD = os.environ.get('DB_MIGRATION_PASS', None) From 04bfa01644a13f063dce75a32a528504faa7f4bd Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Tue, 11 Jun 2013 17:07:59 -0400 Subject: [PATCH 23/55] resolves height issue when display the check and show answer actions alongside problems (cherry picked from commit a17bf5e2ceafdc96fe85ad90a6a39bfc6655329e) --- common/lib/xmodule/xmodule/css/capa/display.scss | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index bd131a9ff6..5dc0bdb171 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -555,10 +555,14 @@ section.problem { @extend .blue-button; } + input.check { + height: ($baseline*2); + } + button.show { height: ($baseline*2); - span { + .show-label { font-size: 1.0em; font-weight: 600; } From a684c6f679f84aed5f769c3223e5d5bd25763ef5 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Tue, 11 Jun 2013 17:10:38 -0400 Subject: [PATCH 24/55] removes HTML elemements from problem action Sass selectors (cherry picked from commit 664efb988756aa2c1a6080f5f29c246c4e7b891b) --- common/lib/xmodule/xmodule/css/capa/display.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 5dc0bdb171..c9f300c966 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -555,11 +555,11 @@ section.problem { @extend .blue-button; } - input.check { + .check { height: ($baseline*2); } - button.show { + .show { height: ($baseline*2); .show-label { From cb46cc6ac6089b35cfdfd1bda07ab80e9c574997 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 12 Jun 2013 10:12:20 -0400 Subject: [PATCH 25/55] abstracts height rule to apply to the capa check, show, save actions (cherry picked from commit 050846af711ca3c86c7a309322c1db0f31973f84) --- common/lib/xmodule/xmodule/css/capa/display.scss | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index c9f300c966..6c61d6dd77 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -551,16 +551,16 @@ section.problem { section.action { margin-top: 20px; - input.save { + .save, .check, .show { + height: ($baseline*2); + font-weight: 600; + } + + .save { @extend .blue-button; } - .check { - height: ($baseline*2); - } - .show { - height: ($baseline*2); .show-label { font-size: 1.0em; From 10fd401157004f8e62671697fe31d6de2ce24fab Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 12 Jun 2013 11:00:15 -0400 Subject: [PATCH 26/55] syncs styling rules for buttons/inputs to ensure consistency (cherry picked from commit 832da3ad48b4e869e369be014c30f71732000679) --- cms/static/sass/_base.scss | 2 +- common/lib/xmodule/xmodule/css/capa/display.scss | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index e0e7543b8e..3713f83ae3 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -14,7 +14,7 @@ body { color: $gray-d2; } -body, input { +body, input, button { font-family: 'Open Sans', sans-serif; } diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 6c61d6dd77..a46dec8d2f 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -553,7 +553,8 @@ section.problem { .save, .check, .show { height: ($baseline*2); - font-weight: 600; + font-weight: 500; + vertical-align: middle; } .save { @@ -564,7 +565,7 @@ section.problem { .show-label { font-size: 1.0em; - font-weight: 600; + font-weight: 500; } } From 45c1109c72d149f7baf530bf9388efffb2976ddc Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 12 Jun 2013 11:04:12 -0400 Subject: [PATCH 27/55] syncs up font-weight across problem button/input actions (cherry picked from commit 14100ba14c8323e61f60775cee7b7d64dc186a45) --- common/lib/xmodule/xmodule/css/capa/display.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index a46dec8d2f..819499cbe5 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -553,7 +553,7 @@ section.problem { .save, .check, .show { height: ($baseline*2); - font-weight: 500; + font-weight: 600; vertical-align: middle; } @@ -565,7 +565,7 @@ section.problem { .show-label { font-size: 1.0em; - font-weight: 500; + font-weight: 600; } } From 86d952bf91c7f5fa3159d2830ef8b81c6ee09c5e Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 12 Jun 2013 13:34:38 -0400 Subject: [PATCH 28/55] Change to putting serialize/de-serialize directly in xml_module. --- .../contentstore/tests/test_checklists.py | 43 +++- .../lib/xmodule/xmodule/tests/test_fields.py | 32 --- .../xmodule/xmodule/tests/test_xml_module.py | 190 +++++++++++++++++- common/lib/xmodule/xmodule/xml_module.py | 40 +++- requirements/edx/github.txt | 3 +- 5 files changed, 261 insertions(+), 47 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index f0889b0861..54bc726092 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -19,6 +19,24 @@ class ChecklistTestCase(CourseTestCase): modulestore = get_modulestore(self.course.location) return modulestore.get_item(self.course.location).checklists + + def compare_checklists(self, persisted, request): + """ + Handles url expansion as possible difference and descends into guts + :param persisted: + :param request: + """ + self.assertEqual(persisted['short_description'], request['short_description']) + compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded')) + for pers, req in zip(persisted['items'], request['items']): + self.assertEqual(pers['short_description'], req['short_description']) + self.assertEqual(pers['long_description'], req['long_description']) + self.assertEqual(pers['is_checked'], req['is_checked']) + if compare_urls: + self.assertEqual(pers['action_url'], req['action_url']) + self.assertEqual(pers['action_text'], req['action_text']) + self.assertEqual(pers['action_external'], req['action_external']) + def test_get_checklists(self): """ Tests the get checklists method. """ checklists_url = get_url_reverse('Checklists', self.course) @@ -31,9 +49,9 @@ class ChecklistTestCase(CourseTestCase): self.course.checklists = None modulestore = get_modulestore(self.course.location) modulestore.update_metadata(self.course.location, own_metadata(self.course)) - self.assertEquals(self.get_persisted_checklists(), None) + self.assertEqual(self.get_persisted_checklists(), None) response = self.client.get(checklists_url) - self.assertEquals(payload, response.content) + self.assertEqual(payload, response.content) def test_update_checklists_no_index(self): """ No checklist index, should return all of them. """ @@ -43,7 +61,8 @@ class ChecklistTestCase(CourseTestCase): 'name': self.course.location.name}) returned_checklists = json.loads(self.client.get(update_url).content) - self.assertListEqual(self.get_persisted_checklists(), returned_checklists) + for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): + self.compare_checklists(pay, resp) def test_update_checklists_index_ignored_on_get(self): """ Checklist index ignored on get. """ @@ -53,7 +72,8 @@ class ChecklistTestCase(CourseTestCase): 'checklist_index': 1}) returned_checklists = json.loads(self.client.get(update_url).content) - self.assertListEqual(self.get_persisted_checklists(), returned_checklists) + for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): + self.compare_checklists(pay, resp) def test_update_checklists_post_no_index(self): """ No checklist index, will error on post. """ @@ -78,13 +98,18 @@ class ChecklistTestCase(CourseTestCase): 'course': self.course.location.course, 'name': self.course.location.name, 'checklist_index': 2}) + + def get_first_item(checklist): + return checklist['items'][0] + payload = self.course.checklists[2] - self.assertFalse(payload.get('is_checked')) - payload['is_checked'] = True + self.assertFalse(get_first_item(payload).get('is_checked')) + get_first_item(payload)['is_checked'] = True returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) - self.assertTrue(returned_checklist.get('is_checked')) - self.assertEqual(self.get_persisted_checklists()[2], returned_checklist) + self.assertTrue(get_first_item(returned_checklist).get('is_checked')) + pers = self.get_persisted_checklists() + self.compare_checklists(pers[2], returned_checklist) def test_update_checklists_delete_unsupported(self): """ Delete operation is not supported. """ @@ -93,4 +118,4 @@ class ChecklistTestCase(CourseTestCase): 'name': self.course.location.name, 'checklist_index': 100}) response = self.client.delete(update_url) - self.assertContains(response, 'Unsupported request', status_code=400) \ No newline at end of file + self.assertContains(response, 'Unsupported request', status_code=400) diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index d944566e97..41b77d4477 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -78,22 +78,6 @@ class DateTest(unittest.TestCase): DateTest.date.from_json("2012-12-31T23:00:01-01:00")), "2013-01-01T00:00:01Z") - def test_serialize(self): - self.assertEqual( - DateTest.date.serialize("2012-12-31T23:59:59Z"), - '"2012-12-31T23:59:59Z"' - ) - - def test_deserialize(self): - self.assertEqual( - '2012-12-31T23:59:59Z', - DateTest.date.deserialize("2012-12-31T23:59:59Z"), - ) - self.assertEqual( - '2012-12-31T23:59:59Z', - DateTest.date.deserialize('"2012-12-31T23:59:59Z"'), - ) - class TimedeltaTest(unittest.TestCase): delta = Timedelta() @@ -114,19 +98,3 @@ class TimedeltaTest(unittest.TestCase): '1 days 46799 seconds', TimedeltaTest.delta.to_json(datetime.timedelta(days=1, hours=12, minutes=59, seconds=59)) ) - - def test_serialize(self): - self.assertEqual( - TimedeltaTest.delta.serialize('1 day 12 hours 59 minutes 59 seconds'), - '"1 day 12 hours 59 minutes 59 seconds"' - ) - - def test_deserialize(self): - self.assertEqual( - '1 day 12 hours 59 minutes 59 seconds', - TimedeltaTest.delta.deserialize('1 day 12 hours 59 minutes 59 seconds') - ) - self.assertEqual( - '1 day 12 hours 59 minutes 59 seconds', - TimedeltaTest.delta.deserialize('"1 day 12 hours 59 minutes 59 seconds"') - ) diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 18cd11650f..c45f14b6c2 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -2,11 +2,12 @@ #pylint: disable=C0111 from xmodule.x_module import XModuleFields -from xblock.core import Scope, String, Object, Boolean, Integer, Float -from xmodule.fields import Date -from xmodule.xml_module import XmlDescriptor +from xblock.core import Scope, String, Object, Boolean, Integer, Float, Any, List +from xmodule.fields import Date, Timedelta +from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field import unittest from .import test_system +from nose.tools import assert_equals from mock import Mock @@ -171,3 +172,186 @@ class EditableMetadataFieldsTest(unittest.TestCase): self.assertEqual(explicitly_set, test_field['explicitly_set']) self.assertEqual(inheritable, test_field['inheritable']) + + +def assertSerializeEqual(expected, arg): + """ + Asserts the result of serialize_field. + """ + assert_equals(expected, serialize_field(arg)) + + +def assertDeserializeEqual(field, expected, arg): + """ + Asserts the result of deserialize_field. + """ + assert_equals(expected, deserialize_field(field, arg)) + + +class TestSerializeInteger(unittest.TestCase): + """ Tests serialize/deserialize as related to Integer type. """ + + def test_serialize(self): + assertSerializeEqual('-2', -2) + assertSerializeEqual('"2"', '2') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Integer(), None, 'null') + assertDeserializeEqual(Integer(), -2, '-2') + assertDeserializeEqual(Integer(), "450", '"450"') + + # False can be parsed as a int (converts to 0) + assertDeserializeEqual(Integer(), False, 'false') + # True can be parsed as a int (converts to 1) + assertDeserializeEqual(Integer(), True, 'true') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(Integer(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class FloatTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Float type. """ + + def test_serialize(self): + assertSerializeEqual('-2', -2) + assertSerializeEqual('"2"', '2') + assertSerializeEqual('-3.41', -3.41) + assertSerializeEqual('"2.589"', '2.589') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Float(), None, 'null') + assertDeserializeEqual(Float(), -2, '-2') + assertDeserializeEqual(Float(), "450", '"450"') + assertDeserializeEqual(Float(), -2.78, '-2.78') + assertDeserializeEqual(Float(), "0.45", '"0.45"') + + # False can be parsed as a float (converts to 0) + assertDeserializeEqual(Float(), False, 'false') + # True can be parsed as a float (converts to 1) + assertDeserializeEqual(Float(), True, 'true') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(Float(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class BooleanTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Boolean type. """ + + def test_serialize(self): + assertSerializeEqual('false', False) + assertSerializeEqual('"false"', 'false') + assertSerializeEqual('"fAlse"', 'fAlse') + assertSerializeEqual('null', None) + + def test_deserialize(self): + # json.loads converts the value to Python bool + assertDeserializeEqual(Boolean(), False, 'false') + assertDeserializeEqual(Boolean(), True, 'true') + + # json.loads fails, string value is returned. + assertDeserializeEqual(Boolean(), 'False', 'False') + assertDeserializeEqual(Boolean(), 'True', 'True') + + # json.loads deserializes 'null' to None + assertDeserializeEqual(Boolean(), None, 'null') + + # json.loads deserializes as a string + assertDeserializeEqual(Boolean(), 'false', '"false"') + assertDeserializeEqual(Boolean(), 'fAlse', '"fAlse"') + assertDeserializeEqual(Boolean(), "TruE", '"TruE"') + + def test_deserialize_unsupported_types(self): + self.assertRaises(TypeError, deserialize_field, None) + + +class StringTest(unittest.TestCase): + """ Tests serialize/deserialize as related to String type. """ + + def test_serialize(self): + assertSerializeEqual('"hat box"', 'hat box') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(String(), 'hAlf', '"hAlf"') + assertDeserializeEqual(String(), 'false', '"false"') + assertDeserializeEqual(String(), 'single quote', 'single quote') + assertDeserializeEqual(String(), None, 'null') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(String(), '3.4', '3.4') + assertDeserializeEqual(String(), 'false', 'false') + assertDeserializeEqual(String(), '2', '2') + assertDeserializeEqual(String(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class AnyTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Any type. """ + + def test_serialize(self): + assertSerializeEqual('{"bar": "hat", "frog": "green"}', {'bar': 'hat', 'frog' : 'green'}) + assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) + assertSerializeEqual('"hat box"', 'hat box') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Any(), 'hAlf', '"hAlf"') + assertDeserializeEqual(Any(), 'false', '"false"') + assertDeserializeEqual(Any(), None, 'null') + assertDeserializeEqual(Any(), {'bar': 'hat', 'frog' : 'green'}, '{"bar": "hat", "frog": "green"}') + assertDeserializeEqual(Any(), [3.5, 5.6], '[3.5, 5.6]') + assertDeserializeEqual(Any(), '[', '[') + assertDeserializeEqual(Any(), False, 'false') + assertDeserializeEqual(Any(), 3.4, '3.4') + + def test_deserialize_unsupported_types(self): + self.assertRaises(TypeError, deserialize_field, None) + + +class ListTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Any type. """ + + def test_serialize(self): + assertSerializeEqual('["foo", "bar"]', ['foo', 'bar']) + assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(List(), ['foo', 'bar'], '["foo", "bar"]') + assertDeserializeEqual(List(), [3.5, 5.6], '[3.5, 5.6]') + assertDeserializeEqual(List(), [], '[]') + assertDeserializeEqual(List(), None, 'null') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(List(), '3.4', '3.4') + assertDeserializeEqual(List(), 'false', 'false') + assertDeserializeEqual(List(), '2', '2') + self.assertRaises(TypeError, deserialize_field, None) + + +class DateTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Date type. """ + + def test_serialize(self): + assertSerializeEqual('"2012-12-31T23:59:59Z"', "2012-12-31T23:59:59Z") + + def test_deserialize(self): + assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', "2012-12-31T23:59:59Z") + assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', '"2012-12-31T23:59:59Z"') + + +class TimedeltaTest(unittest.TestCase): + + def test_serialize(self): + assertSerializeEqual('"1 day 12 hours 59 minutes 59 seconds"', + "1 day 12 hours 59 minutes 59 seconds") + + def test_deserialize(self): + assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + '1 day 12 hours 59 minutes 59 seconds') + assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + '"1 day 12 hours 59 minutes 59 seconds"') diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 56f8b6fd15..0417dfc0a6 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -79,6 +79,42 @@ class AttrMap(_AttrMapBase): return _AttrMapBase.__new__(_cls, from_xml, to_xml) +def serialize_field(value): + """ + Return a string version of the value (where value is the JSON-formatted, internally stored value). + + By default, this is the result of calling json.dumps on the input value. + """ + return json.dumps(value) + + +def deserialize_field(field, value): + """ + Deserialize the string version to the value stored internally. + + Note that this is not the same as the value returned by from_json, as model types typically store + their value internally as JSON. By default, this method will return the result of calling json.loads + on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads + is not supported for this class (see 'is_type_supported'). In either of those cases, this method returns + the input value. + """ + try: + deserialized = json.loads(value) + + if deserialized is None: + return deserialized + try: + field.from_json(deserialized) + return deserialized + except (ValueError, TypeError): + # Support older serialized forms by simply returning the String representation + return value + + except (ValueError, TypeError): + # Support older serialized version, which was just the String (not the result of json.dumps). + return value + + class XmlDescriptor(XModuleDescriptor): """ Mixin class for standardized parsing of from xml @@ -124,8 +160,8 @@ class XmlDescriptor(XModuleDescriptor): def get_map_for_field(cls, attr): for field in set(cls.fields + cls.lms.fields): if field.name == attr: - from_xml = lambda val: field.deserialize(val) - to_xml = lambda val : field.serialize(val) + from_xml = lambda val: deserialize_field(field, val) + to_xml = lambda val : serialize_field(val) return AttrMap(from_xml, to_xml) return AttrMap() diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 36fd9dca06..310807bc55 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,6 +8,7 @@ -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@4c5d2397#egg=XBlock + +-e git+https://github.com/edx/XBlock.git@2abd2937e134e2f78dfa118d6cfd56ec510fd3a4#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From 89d00036038d621699a06d4821addc8123a4d726 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 12 Jun 2013 14:59:17 -0400 Subject: [PATCH 29/55] Be strict about ModelType expected types, rename Object as Dict. --- common/lib/xmodule/xmodule/abtest_module.py | 8 ++++---- common/lib/xmodule/xmodule/capa_module.py | 8 ++++---- common/lib/xmodule/xmodule/course_module.py | 12 ++++++------ common/lib/xmodule/xmodule/peer_grading_module.py | 4 ++-- common/lib/xmodule/xmodule/poll_module.py | 4 ++-- common/lib/xmodule/xmodule/tests/test_xml_module.py | 4 ++-- common/lib/xmodule/xmodule/word_cloud_module.py | 6 +++--- common/lib/xmodule/xmodule/xml_module.py | 4 ++-- requirements/edx/github.txt | 2 +- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 196154df78..2e61076e94 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -6,7 +6,7 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.xml_module import XmlDescriptor from xmodule.exceptions import InvalidDefinitionError -from xblock.core import String, Scope, Object +from xblock.core import String, Scope, Dict DEFAULT = "_DEFAULT_GROUP" @@ -32,9 +32,9 @@ def group_from_value(groups, v): class ABTestFields(object): - group_portions = Object(help="What proportions of students should go in each group", default={DEFAULT: 1}, scope=Scope.content) - group_assignments = Object(help="What group this user belongs to", scope=Scope.preferences, default={}) - group_content = Object(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []}) + group_portions = Dict(help="What proportions of students should go in each group", default={DEFAULT: 1}, scope=Scope.content) + group_assignments = Dict(help="What group this user belongs to", scope=Scope.preferences, default={}) + group_content = Dict(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []}) experiment = String(help="Experiment that this A/B test belongs to", scope=Scope.content) has_children = True diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 392d29134b..fe28c28dac 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -18,7 +18,7 @@ from .progress import Progress from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from xblock.core import Scope, String, Boolean, Object, Integer, Float +from xblock.core import Scope, String, Boolean, Dict, Integer, Float from .fields import Timedelta, Date from xmodule.util.date_utils import time_to_datetime @@ -95,9 +95,9 @@ class CapaFields(object): {"display_name": "Per Student", "value": "per_student"}] ) data = String(help="XML data for the problem", scope=Scope.content) - correct_map = Object(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={}) - input_state = Object(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state) - student_answers = Object(help="Dictionary with the current student responses", scope=Scope.user_state) + correct_map = Dict(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={}) + input_state = Dict(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state) + student_answers = Dict(help="Dictionary with the current student responses", scope=Scope.user_state) done = Boolean(help="Whether the student has answered the problem", scope=Scope.user_state) seed = Integer(help="Random seed for this student", scope=Scope.user_state) weight = Float( diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 063e53aef4..6b7b60259c 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -17,7 +17,7 @@ from xmodule.graders import grader_from_conf from xmodule.util.date_utils import time_to_datetime import json -from xblock.core import Scope, List, String, Object, Boolean +from xblock.core import Scope, List, String, Dict, Boolean from .fields import Date @@ -154,25 +154,25 @@ class CourseFields(object): start = Date(help="Start time when this module is visible", scope=Scope.settings) end = Date(help="Date that this class ends", scope=Scope.settings) advertised_start = String(help="Date that this course is advertised to start", scope=Scope.settings) - grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content) + grading_policy = Dict(help="Grading policy definition for this class", scope=Scope.content) show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings) display_name = String(help="Display name for this module", scope=Scope.settings) tabs = List(help="List of tabs to enable in this course", scope=Scope.settings) end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings) discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings) - discussion_topics = Object( + discussion_topics = Dict( help="Map of topics names to ids", scope=Scope.settings ) - testcenter_info = Object(help="Dictionary of Test Center info", scope=Scope.settings) + testcenter_info = Dict(help="Dictionary of Test Center info", scope=Scope.settings) announcement = Date(help="Date this course is announced", scope=Scope.settings) - cohort_config = Object(help="Dictionary defining cohort configuration", scope=Scope.settings) + cohort_config = Dict(help="Dictionary defining cohort configuration", scope=Scope.settings) is_new = Boolean(help="Whether this course should be flagged as new", scope=Scope.settings) no_grade = Boolean(help="True if this course isn't graded", default=False, scope=Scope.settings) disable_progress_graph = Boolean(help="True if this course shouldn't display the progress graph", default=False, scope=Scope.settings) pdf_textbooks = List(help="List of dictionaries containing pdf_textbook configuration", scope=Scope.settings) html_textbooks = List(help="List of dictionaries containing html_textbook configuration", scope=Scope.settings) - remote_gradebook = Object(scope=Scope.settings) + remote_gradebook = Dict(scope=Scope.settings) allow_anonymous = Boolean(scope=Scope.settings, default=True) allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False) advanced_modules = List(help="Beta modules used in your course", scope=Scope.settings) diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 4dc553458b..fa1b961ece 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -10,7 +10,7 @@ from .x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.modulestore.django import modulestore from .timeinfo import TimeInfo -from xblock.core import Object, String, Scope, Boolean, Integer, Float +from xblock.core import Dict, String, Scope, Boolean, Integer, Float from xmodule.fields import Date from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService @@ -49,7 +49,7 @@ class PeerGradingFields(object): help="The maximum grade that a student can receive for this problem.", default=MAX_SCORE, scope=Scope.settings, values={"min": 0} ) - student_data_for_location = Object( + student_data_for_location = Dict( help="Student data for a given peer grading problem.", scope=Scope.user_state ) diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index dafcef9835..d998c37e2b 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -19,7 +19,7 @@ from xmodule.x_module import XModule from xmodule.stringify import stringify_children from xmodule.mako_module import MakoModuleDescriptor from xmodule.xml_module import XmlDescriptor -from xblock.core import Scope, String, Object, Boolean, List +from xblock.core import Scope, String, Dict, Boolean, List log = logging.getLogger(__name__) @@ -30,7 +30,7 @@ class PollFields(object): voted = Boolean(help="Whether this student has voted on the poll", scope=Scope.user_state, default=False) poll_answer = String(help="Student answer", scope=Scope.user_state, default='') - poll_answers = Object(help="All possible answers for the poll fro other students", scope=Scope.content) + poll_answers = Dict(help="All possible answers for the poll fro other students", scope=Scope.content) answers = List(help="Poll answers from xml", scope=Scope.content, default=[]) question = String(help="Poll question", scope=Scope.content, default='') diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index c45f14b6c2..46cd90b02c 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -2,7 +2,7 @@ #pylint: disable=C0111 from xmodule.x_module import XModuleFields -from xblock.core import Scope, String, Object, Boolean, Integer, Float, Any, List +from xblock.core import Scope, String, Dict, Boolean, Integer, Float, Any, List from xmodule.fields import Date, Timedelta from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field import unittest @@ -22,7 +22,7 @@ class TestFields(object): # Will not be returned by editable_metadata_fields because filtered out by non_editable_metadata_fields. due = Date(scope=Scope.settings) # Will not be returned by editable_metadata_fields because is not Scope.settings. - student_answers = Object(scope=Scope.user_state) + student_answers = Dict(scope=Scope.user_state) # Will be returned, and can override the inherited value from XModule. display_name = String(scope=Scope.settings, default='local default', display_name='Local Display Name', help='local help') diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index 6605f9b870..e87ecf15b9 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -14,7 +14,7 @@ from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule -from xblock.core import Scope, Object, Boolean, List, Integer +from xblock.core import Scope, Dict, Boolean, List, Integer log = logging.getLogger(__name__) @@ -63,11 +63,11 @@ class WordCloudFields(object): scope=Scope.user_state, default=[] ) - all_words = Object( + all_words = Dict( help="All possible words from all students.", scope=Scope.content ) - top_words = Object( + top_words = Dict( help="Top num_top_words words for word cloud.", scope=Scope.content ) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 0417dfc0a6..b27cf97dfa 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -6,7 +6,7 @@ import sys from collections import namedtuple from lxml import etree -from xblock.core import Object, Scope +from xblock.core import Dict, Scope from xmodule.x_module import (XModuleDescriptor, policy_key) from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata @@ -120,7 +120,7 @@ class XmlDescriptor(XModuleDescriptor): Mixin class for standardized parsing of from xml """ - xml_attributes = Object(help="Map of unhandled xml attributes, used only for storage between import and export", + xml_attributes = Dict(help="Map of unhandled xml attributes, used only for storage between import and export", default={}, scope=Scope.settings) # Extension to append to filename paths diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 310807bc55..a6999e1545 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -9,6 +9,6 @@ # Our libraries: --e git+https://github.com/edx/XBlock.git@2abd2937e134e2f78dfa118d6cfd56ec510fd3a4#egg=XBlock +-e git+https://github.com/edx/XBlock.git@3464c0fbfe07bb16791386b26979f66ee2708b06#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From 6e92ddf3ddbd493310217a0f1179ce6c4fa55bca Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 12 Jun 2013 17:07:41 -0400 Subject: [PATCH 30/55] Cannot set String field to a dict anymore! --- .../features/advanced-settings.feature | 7 ++++- .../features/advanced-settings.py | 27 +++++++++++------ .../contentstore/features/problem-editor.py | 6 ++-- cms/djangoapps/contentstore/views/course.py | 7 +++-- .../xmodule/xmodule/tests/test_xml_module.py | 30 +++++++++++++------ common/lib/xmodule/xmodule/xml_module.py | 13 +++++--- 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 558294e890..310c32d05a 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -28,11 +28,16 @@ Feature: Advanced (manual) course policy Scenario: Test how multi-line input appears Given I am on the Advanced Course Settings page in Studio - When I create a JSON object as a value + When I create a JSON object as a value for "discussion_topics" Then it is displayed as formatted And I reload the page Then it is displayed as formatted + Scenario: Test error if value supplied is of the wrong type + Given I am on the Advanced Course Settings page in Studio + When I create a JSON object as a value for "display_name" + Then I get an error on save + Scenario: Test automatic quoting of non-JSON values Given I am on the Advanced Course Settings page in Studio When I create a non-JSON value not in quotes diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index eb00c06ba9..049103db27 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -2,8 +2,7 @@ #pylint: disable=W0621 from lettuce import world, step -from common import * -from nose.tools import assert_false, assert_equal +from nose.tools import assert_false, assert_equal, assert_regexp_matches """ http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdriver.common.keys.html @@ -52,9 +51,9 @@ def edit_the_value_of_a_policy_key_and_save(step): change_display_name_value(step, '"foo"') -@step('I create a JSON object as a value$') -def create_JSON_object(step): - change_display_name_value(step, '{"key": "value", "key_2": "value_2"}') +@step('I create a JSON object as a value for "(.*)"$') +def create_JSON_object(step, key): + change_value(step, key, '{"key": "value", "key_2": "value_2"}') @step('I create a non-JSON value not in quotes$') @@ -82,7 +81,12 @@ def they_are_alphabetized(step): @step('it is displayed as formatted$') def it_is_formatted(step): - assert_policy_entries([DISPLAY_NAME_KEY], ['{\n "key": "value",\n "key_2": "value_2"\n}']) + assert_policy_entries(['discussion_topics'], ['{\n "key": "value",\n "key_2": "value_2"\n}']) + + +@step('I get an error on save$') +def error_on_save(step): + assert_regexp_matches(world.css_text('#notification-error-description'), 'Incorrect setting format') @step('it is displayed as a string') @@ -124,11 +128,16 @@ def get_display_name_value(): def change_display_name_value(step, new_value): + change_value(step, DISPLAY_NAME_KEY, new_value) - world.css_find(".CodeMirror")[get_index_of(DISPLAY_NAME_KEY)].click() + +def change_value(step, key, new_value): + index = get_index_of(key) + world.css_find(".CodeMirror")[index].click() g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") - display_name = get_display_name_value() - for count in range(len(display_name)): + current_value = world.css_find(VALUE_CSS)[index].value + g._element.send_keys(Keys.CONTROL + Keys.END) + for count in range(len(current_value)): g._element.send_keys(Keys.END, Keys.BACK_SPACE) # Must delete "" before typing the JSON value g._element.send_keys(Keys.END, Keys.BACK_SPACE, Keys.BACK_SPACE, new_value) diff --git a/cms/djangoapps/contentstore/features/problem-editor.py b/cms/djangoapps/contentstore/features/problem-editor.py index 5dfcf55046..7679128beb 100644 --- a/cms/djangoapps/contentstore/features/problem-editor.py +++ b/cms/djangoapps/contentstore/features/problem-editor.py @@ -41,7 +41,9 @@ def i_see_five_settings_with_values(step): @step('I can modify the display name') def i_can_modify_the_display_name(step): - world.get_setting_entry(DISPLAY_NAME).find_by_css('.setting-input')[0].fill('modified') + # Verifying that the display name can be a string containing a floating point value + # (to confirm that we don't throw an error because it is of the wrong type). + world.get_setting_entry(DISPLAY_NAME).find_by_css('.setting-input')[0].fill('3.4') verify_modified_display_name() @@ -172,7 +174,7 @@ def verify_modified_randomization(): def verify_modified_display_name(): - world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, 'modified', True) + world.verify_setting_entry(world.get_setting_entry(DISPLAY_NAME), DISPLAY_NAME, '3.4', True) def verify_modified_display_name_with_special_chars(): diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 07f6b9669c..6f3f4ee155 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -401,8 +401,11 @@ def course_advanced_updates(request, org, course, name): request_body.update({'tabs': new_tabs}) #Indicate that tabs should *not* be filtered out of the metadata filter_tabs = False - - response_json = json.dumps(CourseMetadata.update_from_json(location, + try: + response_json = json.dumps(CourseMetadata.update_from_json(location, request_body, filter_tabs=filter_tabs)) + except (TypeError, ValueError), e: + return HttpResponseBadRequest("Incorrect setting format. " + str(e), content_type="text/plain") + return HttpResponse(response_json, mimetype="application/json") diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 46cd90b02c..0eb4a92dfa 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -188,6 +188,18 @@ def assertDeserializeEqual(field, expected, arg): assert_equals(expected, deserialize_field(field, arg)) +def assertDeserializeNonString(field): + """ + Asserts input value is returned for None or something that is not a string. + """ + assertDeserializeEqual(field, None, None) + assertDeserializeEqual(field, 3.14, 3.14) + assertDeserializeEqual(field, True, True) + assertDeserializeEqual(field, [10], [10]) + assertDeserializeEqual(field, {}, {}) + assertDeserializeEqual(field, [], []) + + class TestSerializeInteger(unittest.TestCase): """ Tests serialize/deserialize as related to Integer type. """ @@ -208,7 +220,7 @@ class TestSerializeInteger(unittest.TestCase): def test_deserialize_unsupported_types(self): assertDeserializeEqual(Integer(), '[3]', '[3]') - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(Integer()) class FloatTest(unittest.TestCase): @@ -235,7 +247,7 @@ class FloatTest(unittest.TestCase): def test_deserialize_unsupported_types(self): assertDeserializeEqual(Float(), '[3]', '[3]') - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(Float()) class BooleanTest(unittest.TestCase): @@ -264,8 +276,7 @@ class BooleanTest(unittest.TestCase): assertDeserializeEqual(Boolean(), 'fAlse', '"fAlse"') assertDeserializeEqual(Boolean(), "TruE", '"TruE"') - def test_deserialize_unsupported_types(self): - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(Boolean()) class StringTest(unittest.TestCase): @@ -286,7 +297,7 @@ class StringTest(unittest.TestCase): assertDeserializeEqual(String(), 'false', 'false') assertDeserializeEqual(String(), '2', '2') assertDeserializeEqual(String(), '[3]', '[3]') - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(String()) class AnyTest(unittest.TestCase): @@ -307,9 +318,7 @@ class AnyTest(unittest.TestCase): assertDeserializeEqual(Any(), '[', '[') assertDeserializeEqual(Any(), False, 'false') assertDeserializeEqual(Any(), 3.4, '3.4') - - def test_deserialize_unsupported_types(self): - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(Any()) class ListTest(unittest.TestCase): @@ -330,7 +339,7 @@ class ListTest(unittest.TestCase): assertDeserializeEqual(List(), '3.4', '3.4') assertDeserializeEqual(List(), 'false', 'false') assertDeserializeEqual(List(), '2', '2') - self.assertRaises(TypeError, deserialize_field, None) + assertDeserializeNonString(List()) class DateTest(unittest.TestCase): @@ -342,9 +351,11 @@ class DateTest(unittest.TestCase): def test_deserialize(self): assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', "2012-12-31T23:59:59Z") assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', '"2012-12-31T23:59:59Z"') + assertDeserializeNonString(Date()) class TimedeltaTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Timedelta type. """ def test_serialize(self): assertSerializeEqual('"1 day 12 hours 59 minutes 59 seconds"', @@ -355,3 +366,4 @@ class TimedeltaTest(unittest.TestCase): '1 day 12 hours 59 minutes 59 seconds') assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', '"1 day 12 hours 59 minutes 59 seconds"') + assertDeserializeNonString(Timedelta()) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index b27cf97dfa..9b5de9d7e7 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -95,23 +95,28 @@ def deserialize_field(field, value): Note that this is not the same as the value returned by from_json, as model types typically store their value internally as JSON. By default, this method will return the result of calling json.loads on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads - is not supported for this class (see 'is_type_supported'). In either of those cases, this method returns + is not supported for this class (from_json throws an Error). In either of those cases, this method returns the input value. """ try: deserialized = json.loads(value) - if deserialized is None: return deserialized try: field.from_json(deserialized) return deserialized except (ValueError, TypeError): - # Support older serialized forms by simply returning the String representation + # Support older serialized version, which was just a string, not result of json.dumps. + # If the deserialized version cannot be converted to the type (via from_json), + # just return the original value. For example, if a string value of '3.4' was + # stored for a String field (before we started storing the result of json.dumps), + # then it would be deserialized as 3.4, but 3.4 is not supported for a String + # field. Therefore field.from_json(3.4) will throw an Error, and we should + # actually return the original value of '3.4'. return value except (ValueError, TypeError): - # Support older serialized version, which was just the String (not the result of json.dumps). + # Support older serialized version. return value From 0f8d7c744c75372d5e99e76043cc734ac556f49e Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 13 Jun 2013 10:42:23 -0400 Subject: [PATCH 31/55] Test cleanup. --- .../features/advanced-settings.feature | 2 + .../xmodule/xmodule/tests/test_xml_module.py | 262 +++++++++--------- 2 files changed, 131 insertions(+), 133 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 310c32d05a..13600f2086 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -37,6 +37,8 @@ Feature: Advanced (manual) course policy Given I am on the Advanced Course Settings page in Studio When I create a JSON object as a value for "display_name" Then I get an error on save + And I reload the page + Then the policy key value is unchanged Scenario: Test automatic quoting of non-JSON values Given I am on the Advanced Course Settings page in Studio diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 0eb4a92dfa..e48638d3e8 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -174,196 +174,192 @@ class EditableMetadataFieldsTest(unittest.TestCase): self.assertEqual(inheritable, test_field['inheritable']) -def assertSerializeEqual(expected, arg): - """ - Asserts the result of serialize_field. - """ - assert_equals(expected, serialize_field(arg)) - - -def assertDeserializeEqual(field, expected, arg): - """ - Asserts the result of deserialize_field. - """ - assert_equals(expected, deserialize_field(field, arg)) - - -def assertDeserializeNonString(field): - """ - Asserts input value is returned for None or something that is not a string. - """ - assertDeserializeEqual(field, None, None) - assertDeserializeEqual(field, 3.14, 3.14) - assertDeserializeEqual(field, True, True) - assertDeserializeEqual(field, [10], [10]) - assertDeserializeEqual(field, {}, {}) - assertDeserializeEqual(field, [], []) - - -class TestSerializeInteger(unittest.TestCase): - """ Tests serialize/deserialize as related to Integer type. """ - +class TestSerialize(unittest.TestCase): + """ Tests the serialize, method, which is not dependent on type. """ def test_serialize(self): - assertSerializeEqual('-2', -2) - assertSerializeEqual('"2"', '2') - assertSerializeEqual('null', None) + assert_equals('null', serialize_field(None)) + assert_equals('-2', serialize_field(-2)) + assert_equals('"2"', serialize_field('2')) + assert_equals('-3.41', serialize_field(-3.41)) + assert_equals('"2.589"', serialize_field('2.589')) + assert_equals('false', serialize_field(False)) + assert_equals('"false"', serialize_field('false')) + assert_equals('"fAlse"', serialize_field('fAlse')) + assert_equals('"hat box"', serialize_field('hat box')) + assert_equals('{"bar": "hat", "frog": "green"}', serialize_field({'bar': 'hat', 'frog' : 'green'})) + assert_equals('[3.5, 5.6]', serialize_field([3.5, 5.6])) + assert_equals('["foo", "bar"]', serialize_field(['foo', 'bar'])) + assert_equals('"2012-12-31T23:59:59Z"', serialize_field("2012-12-31T23:59:59Z")) + assert_equals('"1 day 12 hours 59 minutes 59 seconds"', + serialize_field("1 day 12 hours 59 minutes 59 seconds")) + + +class TestDeserialize(unittest.TestCase): + def assertDeserializeEqual(self, expected, arg): + """ + Asserts the result of deserialize_field. + """ + assert_equals(expected, deserialize_field(self.test_field(), arg)) + + + def assertDeserializeNonString(self): + """ + Asserts input value is returned for None or something that is not a string. + For all types, 'null' is also always returned as None. + """ + self.assertDeserializeEqual(None, None) + self.assertDeserializeEqual(3.14, 3.14) + self.assertDeserializeEqual(True, True) + self.assertDeserializeEqual([10], [10]) + self.assertDeserializeEqual({}, {}) + self.assertDeserializeEqual([], []) + self.assertDeserializeEqual(None, 'null') + + +class TestDeserializeInteger(TestDeserialize): + """ Tests deserialize as related to Integer type. """ + + test_field = Integer def test_deserialize(self): - assertDeserializeEqual(Integer(), None, 'null') - assertDeserializeEqual(Integer(), -2, '-2') - assertDeserializeEqual(Integer(), "450", '"450"') + self.assertDeserializeEqual(-2, '-2') + self.assertDeserializeEqual("450", '"450"') # False can be parsed as a int (converts to 0) - assertDeserializeEqual(Integer(), False, 'false') + self.assertDeserializeEqual(False, 'false') # True can be parsed as a int (converts to 1) - assertDeserializeEqual(Integer(), True, 'true') + self.assertDeserializeEqual(True, 'true') + # 2.78 can be converted to int, so the string will be deserialized + self.assertDeserializeEqual(-2.78, '-2.78') + def test_deserialize_unsupported_types(self): - assertDeserializeEqual(Integer(), '[3]', '[3]') - assertDeserializeNonString(Integer()) + self.assertDeserializeEqual('[3]', '[3]') + # '2.78' cannot be converted to int, so input value is returned + self.assertDeserializeEqual('"-2.78"', '"-2.78"') + # 'false' cannot be converted to int, so input value is returned + self.assertDeserializeEqual('"false"', '"false"') + self.assertDeserializeNonString() -class FloatTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Float type. """ +class TestDeserializeFloat(TestDeserialize): + """ Tests deserialize as related to Float type. """ - def test_serialize(self): - assertSerializeEqual('-2', -2) - assertSerializeEqual('"2"', '2') - assertSerializeEqual('-3.41', -3.41) - assertSerializeEqual('"2.589"', '2.589') - assertSerializeEqual('null', None) + test_field = Float def test_deserialize(self): - assertDeserializeEqual(Float(), None, 'null') - assertDeserializeEqual(Float(), -2, '-2') - assertDeserializeEqual(Float(), "450", '"450"') - assertDeserializeEqual(Float(), -2.78, '-2.78') - assertDeserializeEqual(Float(), "0.45", '"0.45"') + self.assertDeserializeEqual( -2, '-2') + self.assertDeserializeEqual("450", '"450"') + self.assertDeserializeEqual(-2.78, '-2.78') + self.assertDeserializeEqual("0.45", '"0.45"') # False can be parsed as a float (converts to 0) - assertDeserializeEqual(Float(), False, 'false') + self.assertDeserializeEqual(False, 'false') # True can be parsed as a float (converts to 1) - assertDeserializeEqual(Float(), True, 'true') + self.assertDeserializeEqual( True, 'true') def test_deserialize_unsupported_types(self): - assertDeserializeEqual(Float(), '[3]', '[3]') - assertDeserializeNonString(Float()) + self.assertDeserializeEqual('[3]', '[3]') + # 'false' cannot be converted to float, so input value is returned + self.assertDeserializeEqual('"false"', '"false"') + self.assertDeserializeNonString() -class BooleanTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Boolean type. """ +class TestDeserializeBoolean(TestDeserialize): + """ Tests deserialize as related to Boolean type. """ - def test_serialize(self): - assertSerializeEqual('false', False) - assertSerializeEqual('"false"', 'false') - assertSerializeEqual('"fAlse"', 'fAlse') - assertSerializeEqual('null', None) + test_field = Boolean def test_deserialize(self): # json.loads converts the value to Python bool - assertDeserializeEqual(Boolean(), False, 'false') - assertDeserializeEqual(Boolean(), True, 'true') + self.assertDeserializeEqual(False, 'false') + self.assertDeserializeEqual(True, 'true') # json.loads fails, string value is returned. - assertDeserializeEqual(Boolean(), 'False', 'False') - assertDeserializeEqual(Boolean(), 'True', 'True') - - # json.loads deserializes 'null' to None - assertDeserializeEqual(Boolean(), None, 'null') + self.assertDeserializeEqual('False', 'False') + self.assertDeserializeEqual('True', 'True') # json.loads deserializes as a string - assertDeserializeEqual(Boolean(), 'false', '"false"') - assertDeserializeEqual(Boolean(), 'fAlse', '"fAlse"') - assertDeserializeEqual(Boolean(), "TruE", '"TruE"') + self.assertDeserializeEqual('false', '"false"') + self.assertDeserializeEqual('fAlse', '"fAlse"') + self.assertDeserializeEqual("TruE", '"TruE"') - assertDeserializeNonString(Boolean()) + # 2.78 can be converted to a bool, so the string will be deserialized + self.assertDeserializeEqual(-2.78, '-2.78') + + self.assertDeserializeNonString() -class StringTest(unittest.TestCase): - """ Tests serialize/deserialize as related to String type. """ +class TestDeserializeString(TestDeserialize): + """ Tests deserialize as related to String type. """ - def test_serialize(self): - assertSerializeEqual('"hat box"', 'hat box') - assertSerializeEqual('null', None) + test_field = String def test_deserialize(self): - assertDeserializeEqual(String(), 'hAlf', '"hAlf"') - assertDeserializeEqual(String(), 'false', '"false"') - assertDeserializeEqual(String(), 'single quote', 'single quote') - assertDeserializeEqual(String(), None, 'null') + self.assertDeserializeEqual('hAlf', '"hAlf"') + self.assertDeserializeEqual('false', '"false"') + self.assertDeserializeEqual('single quote', 'single quote') def test_deserialize_unsupported_types(self): - assertDeserializeEqual(String(), '3.4', '3.4') - assertDeserializeEqual(String(), 'false', 'false') - assertDeserializeEqual(String(), '2', '2') - assertDeserializeEqual(String(), '[3]', '[3]') - assertDeserializeNonString(String()) + self.assertDeserializeEqual('3.4', '3.4') + self.assertDeserializeEqual('false', 'false') + self.assertDeserializeEqual('2', '2') + self.assertDeserializeEqual('[3]', '[3]') + self.assertDeserializeNonString() -class AnyTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Any type. """ +class TestDeserializeAny(TestDeserialize): + """ Tests deserialize as related to Any type. """ - def test_serialize(self): - assertSerializeEqual('{"bar": "hat", "frog": "green"}', {'bar': 'hat', 'frog' : 'green'}) - assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) - assertSerializeEqual('"hat box"', 'hat box') - assertSerializeEqual('null', None) + test_field = Any def test_deserialize(self): - assertDeserializeEqual(Any(), 'hAlf', '"hAlf"') - assertDeserializeEqual(Any(), 'false', '"false"') - assertDeserializeEqual(Any(), None, 'null') - assertDeserializeEqual(Any(), {'bar': 'hat', 'frog' : 'green'}, '{"bar": "hat", "frog": "green"}') - assertDeserializeEqual(Any(), [3.5, 5.6], '[3.5, 5.6]') - assertDeserializeEqual(Any(), '[', '[') - assertDeserializeEqual(Any(), False, 'false') - assertDeserializeEqual(Any(), 3.4, '3.4') - assertDeserializeNonString(Any()) + self.assertDeserializeEqual('hAlf', '"hAlf"') + self.assertDeserializeEqual('false', '"false"') + self.assertDeserializeEqual({'bar': 'hat', 'frog' : 'green'}, '{"bar": "hat", "frog": "green"}') + self.assertDeserializeEqual([3.5, 5.6], '[3.5, 5.6]') + self.assertDeserializeEqual('[', '[') + self.assertDeserializeEqual(False, 'false') + self.assertDeserializeEqual(3.4, '3.4') + self.assertDeserializeNonString() -class ListTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Any type. """ +class TestDeserializeList(TestDeserialize): + """ Tests deserialize as related to List type. """ - def test_serialize(self): - assertSerializeEqual('["foo", "bar"]', ['foo', 'bar']) - assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) - assertSerializeEqual('null', None) + test_field = List def test_deserialize(self): - assertDeserializeEqual(List(), ['foo', 'bar'], '["foo", "bar"]') - assertDeserializeEqual(List(), [3.5, 5.6], '[3.5, 5.6]') - assertDeserializeEqual(List(), [], '[]') - assertDeserializeEqual(List(), None, 'null') + self.assertDeserializeEqual(['foo', 'bar'], '["foo", "bar"]') + self.assertDeserializeEqual([3.5, 5.6], '[3.5, 5.6]') + self.assertDeserializeEqual([], '[]') def test_deserialize_unsupported_types(self): - assertDeserializeEqual(List(), '3.4', '3.4') - assertDeserializeEqual(List(), 'false', 'false') - assertDeserializeEqual(List(), '2', '2') - assertDeserializeNonString(List()) + self.assertDeserializeEqual('3.4', '3.4') + self.assertDeserializeEqual('false', 'false') + self.assertDeserializeEqual('2', '2') + self.assertDeserializeNonString() -class DateTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Date type. """ +class TestDeserializeDate(TestDeserialize): + """ Tests deserialize as related to Date type. """ - def test_serialize(self): - assertSerializeEqual('"2012-12-31T23:59:59Z"', "2012-12-31T23:59:59Z") + test_field = Date def test_deserialize(self): - assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', "2012-12-31T23:59:59Z") - assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', '"2012-12-31T23:59:59Z"') - assertDeserializeNonString(Date()) + self.assertDeserializeEqual('2012-12-31T23:59:59Z', "2012-12-31T23:59:59Z") + self.assertDeserializeEqual('2012-12-31T23:59:59Z', '"2012-12-31T23:59:59Z"') + self.assertDeserializeNonString() -class TimedeltaTest(unittest.TestCase): - """ Tests serialize/deserialize as related to Timedelta type. """ +class TestDeserializeTimedelta(TestDeserialize): + """ Tests deserialize as related to Timedelta type. """ - def test_serialize(self): - assertSerializeEqual('"1 day 12 hours 59 minutes 59 seconds"', - "1 day 12 hours 59 minutes 59 seconds") + test_field = Timedelta def test_deserialize(self): - assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + self.assertDeserializeEqual('1 day 12 hours 59 minutes 59 seconds', '1 day 12 hours 59 minutes 59 seconds') - assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + self.assertDeserializeEqual('1 day 12 hours 59 minutes 59 seconds', '"1 day 12 hours 59 minutes 59 seconds"') - assertDeserializeNonString(Timedelta()) + self.assertDeserializeNonString() From 6af4402abd1f30d784d934a91c523be5aa29f59b Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 13 Jun 2013 10:56:48 -0400 Subject: [PATCH 32/55] Squash 3 more naive datetime warnings. I think the only ones left are in migrations. --- common/djangoapps/student/tests/factories.py | 5 +++-- lms/djangoapps/courseware/tests/test_views.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index d73bb6f01d..49864fcbd4 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -5,6 +5,7 @@ from django.contrib.auth.models import Group from datetime import datetime from factory import DjangoModelFactory, SubFactory, PostGenerationMethodCall, post_generation, Sequence from uuid import uuid4 +from pytz import UTC # Factories don't have __init__ methods, and are self documenting # pylint: disable=W0232 @@ -46,8 +47,8 @@ class UserFactory(DjangoModelFactory): is_staff = False is_active = True is_superuser = False - last_login = datetime(2012, 1, 1) - date_joined = datetime(2011, 1, 1) + last_login = datetime(2012, 1, 1, tzinfo=UTC) + date_joined = datetime(2011, 1, 1, tzinfo=UTC) @post_generation def profile(obj, create, extracted, **kwargs): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 1d3166893e..25492ad379 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -13,6 +13,7 @@ from xmodule.modulestore.django import modulestore import courseware.views as views from xmodule.modulestore import Location +from pytz import UTC class Stub(): @@ -63,7 +64,7 @@ class ViewsTestCase(TestCase): def setUp(self): self.user = User.objects.create(username='dummy', password='123456', email='test@mit.edu') - self.date = datetime.datetime(2013, 1, 22) + self.date = datetime.datetime(2013, 1, 22, tzinfo=UTC) self.course_id = 'edX/toy/2012_Fall' self.enrollment = CourseEnrollment.objects.get_or_create(user=self.user, course_id=self.course_id, From 5735ccaad5ac24c653bf97bc008b3165c18b6684 Mon Sep 17 00:00:00 2001 From: Peter Baratta Date: Thu, 13 Jun 2013 11:09:22 -0400 Subject: [PATCH 33/55] Address Sarina's comments from the PR -Clean up `SUFFIXES` from interspersed commented-out suffixes -Remove unused `LOG` -Remove unused method `raiseself` of UndefinedVariable and its usage --- common/lib/calc/calc.py | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/common/lib/calc/calc.py b/common/lib/calc/calc.py index 3afc0f91bc..27745826c4 100644 --- a/common/lib/calc/calc.py +++ b/common/lib/calc/calc.py @@ -5,7 +5,6 @@ Uses pyparsing to parse. Main function as of now is evaluator(). """ import copy -import logging import math import operator import re @@ -67,15 +66,14 @@ DEFAULT_VARIABLES = {'i': numpy.complex(0, 1), 'q': scipy.constants.e } -# We eliminated extreme ones, since they're rarely used, and potentially +# We eliminated the following extreme suffixes: +# P (1e15), E (1e18), Z (1e21), Y (1e24), +# f (1e-15), a (1e-18), z (1e-21), y (1e-24) +# since they're rarely used, and potentially # confusing. They may also conflict with variables if we ever allow e.g. # 5R instead of 5*R -SUFFIXES = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, - 'T': 1e12, # 'P':1e15,'E':1e18,'Z':1e21,'Y':1e24, - 'c': 1e-2, 'm': 1e-3, 'u': 1e-6, - 'n': 1e-9, 'p': 1e-12} # ,'f':1e-15,'a':1e-18,'z':1e-21,'y':1e-24} - -LOG = logging.getLogger("mitx.courseware.capa") +SUFFIXES = {'%': 0.01, 'k': 1e3, 'M': 1e6, 'G': 1e9, 'T': 1e12, + 'c': 1e-2, 'm': 1e-3, 'u': 1e-6, 'n': 1e-9, 'p': 1e-12} class UndefinedVariable(Exception): @@ -84,10 +82,6 @@ class UndefinedVariable(Exception): instructor. """ pass - # unused for now - # def raiseself(self): - # ''' Helper so we can use inside of a lambda ''' - # raise self def check_variables(string, variables): @@ -96,13 +90,8 @@ def check_variables(string, variables): Otherwise, raise an UndefinedVariable containing all bad variables. - Pyparsing uses a left-to-right parser, which makes the more + Pyparsing uses a left-to-right parser, which makes a more elegant approach pretty hopeless. - - achar = reduce(lambda a,b:a|b ,map(Literal,alphas)) # Any alphabetic character - undefined_variable = achar + Word(alphanums) - undefined_variable.setParseAction(lambda x:UndefinedVariable("".join(x)).raiseself()) - varnames = varnames | undefined_variable """ general_whitespace = re.compile('[^\\w]+') # List of all alnums in string @@ -235,9 +224,6 @@ def evaluator(variables, functions, string, cs=False): cs: Case sensitive """ - # LOG.debug("variables: {0}".format(variables)) - # LOG.debug("functions: {0}".format(functions)) - # LOG.debug("string: {0}".format(string)) all_variables = copy.copy(DEFAULT_VARIABLES) all_functions = copy.copy(DEFAULT_FUNCTIONS) From c7e37887b34367f4afce5c9aaa735249abcdd0d1 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 13 Jun 2013 11:22:34 -0400 Subject: [PATCH 34/55] Merge updates. --- common/lib/xmodule/xmodule/capa_module.py | 2 +- common/lib/xmodule/xmodule/peer_grading_module.py | 4 ++-- common/lib/xmodule/xmodule/tests/test_xml_module.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index cdb8ce0710..5779fc01c4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -315,7 +315,7 @@ class CapaModule(CapaFields, XModule): # If the user has forced the save button to display, # then show it as long as the problem is not closed # (past due / too many attempts) - if self.force_save_button == "true": + if self.force_save_button: return not self.closed() else: is_survey_question = (self.max_attempts == 0) diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 1d910c121b..b56e089732 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -54,7 +54,7 @@ class PeerGradingFields(object): help="Student data for a given peer grading problem.", scope=Scope.user_state ) - weight = StringyFloat( + weight = Float( display_name="Problem Weight", help="Defines the number of points each problem is worth. If the value is not set, each problem is worth one point.", scope=Scope.settings, values={"min": 0, "step": ".1"} @@ -112,7 +112,7 @@ class PeerGradingModule(PeerGradingFields, XModule): if not self.ajax_url.endswith("/"): self.ajax_url = self.ajax_url + "/" - # StringyInteger could return None, so keep this check. + # Integer could return None, so keep this check. if not isinstance(self.max_grade, int): raise TypeError("max_grade needs to be an integer.") diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index e48638d3e8..eb715b44bc 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -105,7 +105,7 @@ class EditableMetadataFieldsTest(unittest.TestCase): def test_type_and_options(self): # test_display_name_field verifies that a String field is of type "Generic". - # test_integer_field verifies that a StringyInteger field is of type "Integer". + # test_integer_field verifies that a Integer field is of type "Integer". descriptor = self.get_descriptor({}) editable_fields = descriptor.editable_metadata_fields From 9d0867ea11c908eeb2a709ea11653778b5120712 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 13 Jun 2013 11:31:10 -0400 Subject: [PATCH 35/55] Merge cleanup. --- common/lib/xmodule/xmodule/fields.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 5a48bc23ac..963b70204e 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -2,12 +2,10 @@ import time import logging import re -from datetime import timedelta from xblock.core import ModelType import datetime import dateutil.parser -from xblock.core import Integer, Float, Boolean from django.utils.timezone import UTC log = logging.getLogger(__name__) From dac174e4488b240f51785a5c2e077e00a508f83e Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 13 Jun 2013 11:31:18 -0400 Subject: [PATCH 36/55] Merge cleanup. --- requirements/edx/github.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index a6999e1545..fab0c85d7f 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -9,6 +9,6 @@ # Our libraries: --e git+https://github.com/edx/XBlock.git@3464c0fbfe07bb16791386b26979f66ee2708b06#egg=XBlock --e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail --e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover +-e git+https://github.com/edx/XBlock.git@3464c0fbfe#egg=XBlock +-e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail +-e git+https://github.com/edx/diff-cover.git@v0.1.1#egg=diff_cover From 8508e3f647c51014d70402a20932bc77ada7a36f Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Wed, 12 Jun 2013 17:45:31 -0400 Subject: [PATCH 37/55] dummy object creation without conditionals --- lms/templates/widgets/segment-io.html | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lms/templates/widgets/segment-io.html b/lms/templates/widgets/segment-io.html index 6b4ace8375..7d979c989f 100644 --- a/lms/templates/widgets/segment-io.html +++ b/lms/templates/widgets/segment-io.html @@ -1,7 +1,8 @@ -% if settings.MITX_FEATURES.get('SEGMENT_IO_LMS'): -% else: - - - -% endif From d136fdac9a7c4ea2e1e6bed0105eb18844e8b1c2 Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Thu, 13 Jun 2013 10:39:05 -0400 Subject: [PATCH 38/55] added comment to make clear that leaving the variable analytics outside of the feature flag block is intentional --- lms/templates/widgets/segment-io.html | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/templates/widgets/segment-io.html b/lms/templates/widgets/segment-io.html index 7d979c989f..dea222653e 100644 --- a/lms/templates/widgets/segment-io.html +++ b/lms/templates/widgets/segment-io.html @@ -1,5 +1,6 @@