PR cleanup

- Percentile range (0,100) -> [0,100]
- Adjust string validation error message
- make PARAMS a classmethod instead to not leak
  parent class name outside its definition

django/climate_change_api/indicators/params.py

@@ -17,7 +17,10 @@ class IndicatorParams(object):
     After calling validate(), the validated parameter values are available as instance attributes.
 
     """
-    PARAMS = ('models', 'years', 'agg', 'units',)
+
+    @classmethod
+    def params(cls):
+        return ('models', 'years', 'agg', 'units',)
 
     def __init__(self, query_params, available_units, default_units):
         """
@@ -27,15 +30,15 @@ def __init__(self, query_params, available_units, default_units):
         @param default_units (string): Default value for 'units' to set if none provided
                                        in query_params
         """
-        self._params = query_params
+        self._raw_params = query_params
         self._available_units = available_units
         self._default_units = default_units
         self.validated = False
 
     def validate(self):
         """ Validate all parameters found in self.PARAMS """
-        for param in self.PARAMS:
-            value = self._params.get(param, None)
+        for param in self.params():
+            value = self._raw_params.get(param, None)
             validation_fn = getattr(self, 'validate_' + param)
             clean_value = validation_fn(value)
             setattr(self, param, clean_value)
@@ -61,27 +64,29 @@ def __repr__(self):
         """ Override repr to improve class self-documentation """
         representation = {
             'validated': self.validated,
-            'PARAMS': self.PARAMS,
+            'params': self.params,
         }
         if self.validated:
-            for p in self.PARAMS:
+            for p in self.params():
                 representation[p] = getattr(self, p)
         else:
-            for k, v in self._params.iteritems():
+            for k, v in self._raw_params.iteritems():
                 representation[k] = v
         return str(representation)
 
     def _check_is_string(self, param, value):
         """ Private helper method to raise ValidationError if value is not a string """
         if not isinstance(value, basestring):
-            raise ValidationError('{} must be a string comma separated list'.format(param))
+            raise ValidationError('{} must be a string'.format(param))
         return value
 
 
 class PercentileIndicatorParams(IndicatorParams):
     """ Extend IndicatorParams with a non-optional 'percentile' parameter """
 
-    PARAMS = IndicatorParams.PARAMS + ('percentile',)
+    @classmethod
+    def params(cls):
+        return super(PercentileIndicatorParams, cls).params() + ('percentile',)
 
     def validate_percentile(self, value):
         if value is None:
@@ -90,6 +95,6 @@ def validate_percentile(self, value):
             int_value = int(value)
         except ValueError:
             raise ValidationError('percentile param must be an integer')
-        if int_value < 1 or int_value > 99:
-            raise ValidationError('percentile param must be within the range (0-100)')
+        if int_value < 0 or int_value > 100:
+            raise ValidationError('percentile param must be within the range [0-100]')
         return int_value

django/climate_change_api/indicators/tests/test_indicator_params.py

@@ -60,13 +60,13 @@ def test_validate_percentile_none(self):
             indicator_params.validate()
 
     def test_validate_percentile_out_of_bounds(self):
-        """ It should raise ValidationError if percentile outside range 1-99 """
-        parameters = merge_dicts(self.default_parameters, {'percentile': '0'})
+        """ It should raise ValidationError if percentile outside range [0-100] """
+        parameters = merge_dicts(self.default_parameters, {'percentile': '-1'})
         indicator_params = self.params_class(parameters, ('K',), 'K')
         with self.assertRaises(params.ValidationError):
             indicator_params.validate()
 
-        parameters = merge_dicts(self.default_parameters, {'percentile': '100'})
+        parameters = merge_dicts(self.default_parameters, {'percentile': '101'})
         indicator_params = self.params_class(parameters, ('K',), 'K')
         with self.assertRaises(params.ValidationError):
             indicator_params.validate()