Merge lp:~methanal-developers/methanal/float-input into lp:methanal

[Jonathan Jacobs]

[Tristan Seligmann (mithrandi)]

48 + * L{Methanal.Util.strToFloat} converts a float value, represented
49 + * as a C{String}, to a C{Float}.

This docstring can't be accurate, as there is no such thing as Float in JavaScript.

367 + self._validInput = /^[-+]?\d*(\.\d*)?$/;

This regex could also be written as /^[-+]?\d*\.?\d*$/ but I can't really decide which one is easier to understand.

348 } else if (value === undefined || isNaN(value)) {
349 return 'Numerical value only';
350 - } else if (value <= self.lowerBound) {
351 + } else if (value !== undefined && value <= self.lowerBound) {
352 return 'Value too small; must be greater than ' + self.lowerBound;
353 - } else if (value >= self.upperBound) {
354 + } else if (value !== undefined && value >= self.upperBound) {

These changes look pointless; if value === undefined, the earlier case will match, thus by the time we reach the latter two, value !== undefined must be true.

review: Needs Fixing

[lp:~methanal-developers/methanal/float-input]

162. By Jonathan Jacobs on 2011-02-18

Correct docstrings.

163. By Jonathan Jacobs on 2011-02-18

Slightly improve floating point regex.

164. By Jonathan Jacobs on 2011-02-18

Remove pointless checks.

[Jonathan Jacobs (jjacobs)]

> This docstring can't be accurate, as there is no such thing as Float in
> JavaScript.

Fixed.

> This regex could also be written as /^[-+]?\d*\.?\d*$/ but I can't really
> decide which one is easier to understand.

I think your version is slightly more readable.

> These changes look pointless; if value === undefined, the earlier case will
> match, thus by the time we reach the latter two, value !== undefined must be
> true.

Fixed.

[Tristan Seligmann (mithrandi)]

review: Approve

=== modified file 'methanal/js/Methanal/Tests/TestUtil.js'
--- methanal/js/Methanal/Tests/TestUtil.js	2011-02-13 18:50:04 +0000
+++ methanal/js/Methanal/Tests/TestUtil.js	2011-02-18 09:27:38 +0000
@@ -180,6 +180,26 @@
 
 
     /**
+     * Return C{true} iff the given string represents a base-10 numerical value.
+     */
+    function test_isNumericalString(self) {
+        var CASES = [
+            ['1234',    true],
+            ['1.23',    true],
+            ['0',       true],
+            ['01',      true],
+            [1234,      false],
+            [1.23,      false],
+            ['0x1',     false],
+            ['abc',     false],
+            [null,      false],
+            [undefined, false]];
+        Methanal.Tests.Util.assertCases(
+            self, Methanal.Util.isNumericalString, CASES);
+    },
+
+
+    /**
      * L{Methanal.Util.strToInt} converts a base-10 integer value, represented
      * as a C{String}, to an C{Integer}.
      */
@@ -192,14 +212,25 @@
             ['123abc', undefined],
             ['abc123', undefined],
             ['0x123', undefined]];
-
-        for (var i = 0; i < CASES.length; ++i) {
-            var input = CASES[i][0];
-            var expected = CASES[i][1];
-            var actual = Methanal.Util.strToInt(input);
-            self.assert(expected === actual, 'input = ' + input +
-                ' :: expected = ' + expected + ' :: actual = ' + actual);
-        }
+        Methanal.Tests.Util.assertCases(self, Methanal.Util.strToInt, CASES);
+    },
+
+
+    /**
+     * L{Methanal.Util.strToFloat} converts a float value, represented
+     * as a C{String}, to a floating point C{Number}.
+     */
+    function test_strToFloat(self) {
+        var CASES = [
+            ['1234',      1234],
+            ['01234.56',  1234.56],
+            ['.0',        0],
+            ['.5',        0.5],
+            ['-1',        -1],
+            ['123.45abc', undefined],
+            ['abc123.45', undefined],
+            ['0x123',     undefined]];
+        Methanal.Tests.Util.assertCases(self, Methanal.Util.strToFloat, CASES);
     },
 
 

=== modified file 'methanal/js/Methanal/Tests/TestView.js'
--- methanal/js/Methanal/Tests/TestView.js	2010-11-19 12:32:22 +0000
+++ methanal/js/Methanal/Tests/TestView.js	2011-02-18 09:27:38 +0000
@@ -1006,6 +1006,34 @@
 
 
     /**
+     * L{Methanal.View.IntegerInput.getValue} returns an integer value if the
+     * input node's value is a valid number, C{null} if it is blank and
+     * C{undefined} if the value is invalid.
+     */
+    function test_getValue(self) {
+        var CASES = [
+            [null,  null],
+            ['',    null],
+            ['abc', undefined],
+            ['0.5', undefined],
+            ['1a',  undefined],
+            ['0',   0],
+            ['-1',  -1],
+            ['42',  42]];
+
+        self.testControl({value: null},
+            function (control) {
+                Methanal.Tests.Util.assertCases(
+                    self,
+                    function (value) {
+                        control.setValue(value);
+                        return control.getValue();
+                    }, CASES);
+            });
+    },
+
+
+    /**
      * L{Methanal.View.IntegerInput} only accepts input that is a valid integer.
      */
     function test_inputValidation(self) {
@@ -1063,6 +1091,115 @@
 
 
 /**
+ * Tests for L{Methanal.View.FloatInput}.
+ */
+Methanal.Tests.TestView.BaseTestTextInput.subclass(
+    Methanal.Tests.TestView, 'TestFloatInput').methods(
+    function setUp(self) {
+        self.controlType = Methanal.View.FloatInput;
+    },
+
+
+    /**
+     * L{Methanal.View.FloatInput.getValue} returns an float value if the
+     * input node's value is a valid number, C{null} if it is blank and
+     * C{undefined} if the value is invalid.
+     */
+    function test_getValue(self) {
+        var CASES = [
+            [null,  null],
+            ['',    null],
+            ['abc', undefined],
+            ['0.5', 0.5],
+            ['.5',  0.5],
+            ['-.5', -0.5],
+            ['1a',  undefined],
+            ['0',   0],
+            ['-1',  -1],
+            ['42',  42],
+            ['1.2', 1.2]];
+
+        self.testControl({value: null},
+            function (control) {
+                Methanal.Tests.Util.assertCases(
+                    self,
+                    function (value) {
+                        control.setValue(value);
+                        return control.getValue();
+                    }, CASES);
+            });
+    },
+
+
+    /**
+     * L{Methanal.View.FloatInput} only accepts input that is a valid float.
+     */
+    function test_inputValidation(self) {
+        self.testControl({value: null},
+            function (control) {
+                self.assertValidInput(control, null);
+                self.assertValidInput(control, '');
+                self.assertValidInput(control, '1');
+                self.assertValidInput(control, '1.1');
+                self.assertValidInput(control, '+1.0');
+                self.assertValidInput(control, '1.');
+                self.assertValidInput(control, '-1.0');
+                self.assertValidInput(control, '.1');
+                self.assertValidInput(control, '+.1');
+                self.assertValidInput(control, '-.1');
+                self.assertValidInput(control, '.0');
+                self.assertInvalidInput(control, 'a');
+                self.assertInvalidInput(control, '1.2.1');
+            });
+    },
+
+
+    /**
+     * L{Methanal.View.FloatInput} validates that values fall within a certain
+     * exclusive range.
+     */
+    function test_bounds(self) {
+        self.testControl({value: null,
+                          lowerBound: -11,
+                          upperBound:   8,},
+            function (control) {
+                self.assertValidInput(control, '-10.99');
+                self.assertInvalidInput(control, '-11.0');
+                self.assertValidInput(control, '7.99');
+                self.assertInvalidInput(control, '8.0');
+            });
+    },
+
+
+    /**
+     * L{Methanal.View.FloatInput} validates that values fall within a certain
+     * exclusive range, even for really big numbers that have precision
+     * problems in Javascript.
+     */
+    function test_bigBounds(self) {
+        // These turn into -9223372036854776000 and 9223372036854776000.
+        self.testControl({value: null,
+                          decimalPlaces: 2,
+                          lowerBound: -9223372036854775809.5,
+                          upperBound:  9223372036854775808.5},
+            function (control) {
+                self.assertValidInput(control, null);
+                self.assertValidInput(control, '');
+                self.assertValidInput(control, '1');
+                self.assertInvalidInput(control, '-9223372036854775800');
+                self.assertInvalidInput(control, '-9223372036854775809');
+                self.assertInvalidInput(control, '-9223372036854775808.5');
+                self.assertInvalidInput(control, '-92233720368547758090');
+                self.assertInvalidInput(control,  '9223372036854775800');
+                self.assertInvalidInput(control,  '9223372036854775808');
+                self.assertInvalidInput(control,  '9223372036854775807.5');
+                self.assertInvalidInput(control,  '92233720368547758080');
+            });
+    });
+
+
+
+/**
  * Tests for L{Methanal.View.DecimalInput}.
  */
 Methanal.Tests.TestView.BaseTestTextInput.subclass(

=== modified file 'methanal/js/Methanal/Tests/TestWidgets.js'
--- methanal/js/Methanal/Tests/TestWidgets.js	2010-07-11 19:17:21 +0000
+++ methanal/js/Methanal/Tests/TestWidgets.js	2011-02-18 09:27:38 +0000
@@ -276,6 +276,25 @@
 
 
 /**
+ * Tests for L{Methanal.Widgets.FloatColumn}.
+ */
+Methanal.Tests.TestWidgets.SimpleColumnTest.subclass(
+    Methanal.Tests.TestWidgets, 'TestFloatColumn').methods(
+    function setUp(self) {
+        self.columnType = Methanal.Widgets.FloatColumn;
+        self.columnValues = {
+            'a': {value: 42.0,
+                  link: null,
+                  nodeValue: '42'},
+            'b': {value: 1.23,
+                  link: 'quux',
+                  nodeValue: '1.23'}};
+        Methanal.Tests.TestWidgets.TestFloatColumn.upcall(self, 'setUp');
+    });
+
+
+
+/**
  * Tests for L{Methanal.Widgets.BooleanColumn}.
  */
 Methanal.Tests.TestWidgets.SimpleColumnTest.subclass(

=== modified file 'methanal/js/Methanal/Tests/Util.js'
--- methanal/js/Methanal/Tests/Util.js	2010-07-23 20:07:38 +0000
+++ methanal/js/Methanal/Tests/Util.js	2011-02-18 09:27:38 +0000
@@ -111,3 +111,21 @@
     }
     throw new Error('Setter properties not supported!');
 }
+
+
+/**
+ * Assert that an C{Array} of input and expected pairs (C{[input,
+ * expectedOutput]}) match when the input is processed by C{fn}.
+ */
+Methanal.Tests.Util.assertCases = function assertCases(testcase, fn, cases) {
+    for (var i = 0; i < cases.length; ++i) {
+        var input = cases[i][0];
+        var expected = cases[i][1];
+        var actual = fn(input);
+        var repr = Methanal.Util.repr;
+        testcase.assert(
+            expected === actual,
+            'input = ' + repr(input) + ' :: expected = ' + repr(expected) +
+            ' :: actual = ' + repr(actual));
+    }
+};

=== modified file 'methanal/js/Methanal/Util.js'
--- methanal/js/Methanal/Util.js	2011-02-13 18:50:04 +0000
+++ methanal/js/Methanal/Util.js	2011-02-18 09:27:38 +0000
@@ -133,20 +133,32 @@
 
 
 /**
+ * Return C{true} iff the given string represents a base-10 numerical value.
+ */
+Methanal.Util.isNumericalString = function isNumericalString(s) {
+    if (typeof s !== 'string') {
+        return false;
+    } else if (s.indexOf('x') !== -1) {
+        return false;
+    } else if (isNaN(s)) {
+        return false;
+    }
+    return true;
+};
+
+
+
+/**
  * Convert a string to a base-10 integer.
  *
  * Not quite as simple to do properly as one might think.
  *
  * @type s: C{String}
  *
- * @rtype: C{Integer}
+ * @rtype: C{Number} or C{undefined}
  */
 Methanal.Util.strToInt = function strToInt(s) {
-    if (typeof s !== 'string') {
-        return undefined;
-    } else if (s.indexOf('x') !== -1) {
-        return undefined;
-    } else if (isNaN(s)) {
+    if (!Methanal.Util.isNumericalString(s)) {
         return undefined;
     }
     return parseInt(s, 10);
@@ -155,6 +167,22 @@
 
 
 /**
+ * Convert a string to a floating point C{Number}.
+ *
+ * @type s: C{String}
+ *
+ * @rtype: C{Number} or C{undefined}
+ */
+Methanal.Util.strToFloat = function strToFloat(s) {
+    if (!Methanal.Util.isNumericalString(s)) {
+        return undefined;
+    }
+    return parseFloat(s);
+};
+
+
+
+/**
  * Pretty print a decimal number with thousands separators.
  *
  * Useful for formatting large currency amounts in a human-readable way.

=== modified file 'methanal/js/Methanal/View.js'
--- methanal/js/Methanal/View.js	2010-11-22 17:37:26 +0000
+++ methanal/js/Methanal/View.js	2011-02-18 09:27:38 +0000
@@ -2364,6 +2364,26 @@
 
 
 /**
+ * Float input.
+ */
+Methanal.View.NumericInput.subclass(Methanal.View, 'FloatInput').methods(
+    function __init__(self, node, args) {
+        Methanal.View.FloatInput.upcall(self, '__init__', node, args);
+        self._validInput = /^[-+]?\d*\.?\d*$/;
+    },
+
+
+    function getValue(self) {
+        var value = Methanal.View.FloatInput.upcall(self, 'getValue');
+        if (value) {
+            value = Methanal.Util.strToFloat(value);
+        }
+        return value;
+    });
+
+
+
+/**
  * Decimal number input.
  *
  * @type decimalPlaces: C{Integer}

=== modified file 'methanal/js/Methanal/Widgets.js'
--- methanal/js/Methanal/Widgets.js	2010-11-17 15:58:21 +0000
+++ methanal/js/Methanal/Widgets.js	2011-02-18 09:27:38 +0000
@@ -187,6 +187,13 @@
 
 
 /**
+ * Column representing float values.
+ */
+Methanal.Widgets.Column.subclass(Methanal.Widgets, 'FloatColumn');
+
+
+
+/**
  * Column representing boolean values.
  */
 Methanal.Widgets.Column.subclass(Methanal.Widgets, 'BooleanColumn');

=== modified file 'methanal/model.py'
--- methanal/model.py	2010-02-22 21:29:52 +0000
+++ methanal/model.py	2011-02-18 09:27:38 +0000
@@ -279,11 +279,12 @@
 
 
 _paramTypes = {
-    attributes.integer:   Value,
-    attributes.text:      Value,
-    attributes.boolean:   Value,
-    attributes.textlist:  List,
-    attributes.timestamp: Value}
+    attributes.integer:        Value,
+    attributes.ieee754_double: Value,
+    attributes.text:           Value,
+    attributes.boolean:        Value,
+    attributes.textlist:       List,
+    attributes.timestamp:      Value}
 
 def paramFromAttribute(store, attr, value, name=None):
     doc = attr.doc or None

=== modified file 'methanal/test/test_view.py'
--- methanal/test/test_view.py	2010-07-17 20:16:49 +0000
+++ methanal/test/test_view.py	2011-02-18 09:27:38 +0000
@@ -342,6 +342,42 @@
 
 
 
+class FloatInputTests(FormInputTests, NumericInputTestsMixin):
+    """
+    Tests for L{methanal.view.FloatInput}.
+    """
+    controlType = view.FloatInput
+
+
+    def test_getValue(self):
+        """
+        L{methanal.view.FloatInput.getValue} retrieves an empty string in the
+        C{None} case and a C{float} in the case where a value exists.
+        """
+        control = self.createControl(dict())
+        param = control.parent.param
+
+        param.value = None
+        self.assertEquals(control.getValue(), u'')
+
+        param.value = u'5'
+        self.assertTrue(isinstance(control.getValue(), float))
+        self.assertEquals(control.getValue(), 5.0)
+        param.value = 5.0
+        self.assertTrue(isinstance(control.getValue(), float))
+        self.assertEquals(control.getValue(), 5.0)
+
+
+    def test_minimumMaximumDefaultValues(self):
+        """
+        The limits for SQL and Python float types are the same value, so no
+        minimum or maximum checking is required in Methanal for the default
+        ranges.
+        """
+        pass
+
+
+
 class DecimalInputTests(FormInputTests, NumericInputTestsMixin):
     """
     Tests for L{methanal.view.DecimalInput}.

=== modified file 'methanal/view.py'
--- methanal/view.py	2010-09-10 13:41:41 +0000
+++ methanal/view.py	2011-02-18 09:27:38 +0000
@@ -9,7 +9,7 @@
 from twisted.python.versions import Version
 from twisted.python.deprecate import deprecated
 
-from axiom.attributes import text, integer, timestamp, boolean
+from axiom.attributes import text, integer, timestamp, boolean, ieee754_double
 
 from nevow.page import renderer
 from nevow.athena import expose
@@ -649,6 +649,8 @@
     MINIMUM = -9223372036854775808
     MAXIMUM =  9223372036854775807
 
+    NO_BOUNDS = object()
+
 
     def __init__(self, minimumValue=None, maximumValue=None, **kw):
         super(NumericInput, self).__init__(**kw)
@@ -662,9 +664,12 @@
 
 
     def getArgs(self):
-        return {
-            u'lowerBound': self.minimumValue - 1,
-            u'upperBound': self.maximumValue + 1}
+        args = {}
+        if self.minimumValue is not self.NO_BOUNDS:
+            args[u'lowerBound'] = self.minimumValue - 1
+        if self.maximumValue is not self.NO_BOUNDS:
+            args[u'upperBound'] = self.maximumValue + 1
+        return args
 
 
     def checkValue(self, value):
@@ -674,12 +679,13 @@
         """
         if value is None:
             return
-        if value > self.maximumValue:
+        maximumValue, minimumValue = self.maximumValue, self.minimumValue
+        if maximumValue is not self.NO_BOUNDS and value > maximumValue:
             raise ValueError('%s is larger than %s' % (
-                value, self.maximumValue))
-        elif value < self.minimumValue:
+                value, maximumValue))
+        elif minimumValue is not self.NO_BOUNDS and value < minimumValue:
             raise ValueError('%s is smaller than %s' % (
-                value, self.minimumValue))
+                value, minimumValue))
 
 
     def convertValue(self, value):
@@ -708,6 +714,33 @@
 
 
 
+class FloatInput(NumericInput):
+    """
+    Float input.
+    """
+    jsClass = u'Methanal.View.FloatInput'
+
+
+    def __init__(self, minimumValue=NumericInput.NO_BOUNDS,
+                 maximumValue=NumericInput.NO_BOUNDS, **kw):
+        super(FloatInput, self).__init__(
+            minimumValue=minimumValue, maximumValue=maximumValue, **kw)
+
+
+    def convertValue(self, value):
+        if value is not None:
+            value = float(value)
+        return value
+
+
+    def getValue(self):
+        value = self.param.value
+        if value is None:
+            return u''
+        return float(value)
+
+
+
 class DecimalInput(NumericInput):
     """
     Decimal input.
@@ -1320,10 +1353,11 @@
 
 
 _inputTypes = {
-    boolean:    lambda env: CheckboxInput,
-    text:       lambda env: TextInput,
-    integer:    lambda env: IntegerInput,
-    timestamp:  lambda env:
+    boolean:        lambda env: CheckboxInput,
+    text:           lambda env: TextInput,
+    integer:        lambda env: IntegerInput,
+    ieee754_double: lambda env: FloatInput,
+    timestamp:      lambda env:
         lambda **kw: DateInput(timezone=env['timezone'], **kw)}
 
 def inputTypeFromAttribute(attr, **env):

=== modified file 'methanal/widgets.py'
--- methanal/widgets.py	2010-04-25 13:48:54 +0000
+++ methanal/widgets.py	2011-02-18 09:27:38 +0000
@@ -211,6 +211,7 @@
     columnTypes = {
         'text': u'Methanal.Widgets.TextColumn',
         'integer': u'Methanal.Widgets.IntegerColumn',
+        'ieee754_double': u'Methanal.Widgets.FloatColumn',
         'boolean': u'Methanal.Widgets.BooleanColumn',
         'timestamp': u'Methanal.Widgets.TimestampColumn'}