Move string fields, get rid of hard-coded list of booleans.

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)
-

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()

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"}

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

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(

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)
-

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)
 

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,

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

lms/xmodule_namespace.py

 """
 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

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