From 0baec0a164fda2881f043acf0d7f83dfe99e9a58 Mon Sep 17 00:00:00 2001 From: cahrens Date: Fri, 7 Jun 2013 15:45:34 -0400 Subject: [PATCH] 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