Merge pull request #137 from lisad/fix-column-blank-param

Cleans up the 'blank' param so it's only on string column, adds test,…

Author: lisad

phaser/column.py

@@ -17,10 +17,7 @@
 Column: Use for strings & general purpose
 IntColumn, FloatColumn: Cast to int/float datatypes and apply number logic like min_value, max_value
 DateColumn, DateTimeColumn: Cast to date/time datatypes and apply calendar logic like default timezone
-
-TODOs:
-* Probably the default column and the other columns should both inherit from a BaseColumn, so we can
-  have fields like 'blank' on Column but not on IntColumn
+BooleanColumn: cast 1, 0, T, F, yes, no, TRUE, FALSE etc to python True and False values.
 
 """
 
@@ -83,7 +80,7 @@ def __init__(self,
             self.use_exception = Column.ON_ERROR_VALUES[on_error]
 
         if self.null is False and self.default is not None:
-            raise Exception(f"Column {self.name} defined to error on null values, but also provides a non-null default")
+            raise PhaserError(f"Column {self.name} defined to error on null values, but also provides a non-null default")
 
     def check_required(self, data_headers):
         """
@@ -128,7 +125,7 @@ def check_value(self, value):
         """ Raises chosen exception type if something is wrong with a value in the column.
             One can override this to use a different exception or check value in a different way
             (don't forget to call super().check_value() """
-        if not self.blank and not value:  # Python boolean casting returns false if string is empty
+        if not self.blank and not value.strip():  # Python boolean casting returns false if string is empty
             raise self.use_exception(f"Column `{self.name}' had blank value")
         if self.allowed_values and not (value in self.allowed_values):
             raise self.use_exception(f"Column '{self.name}' had value {value} not found in allowed values")
@@ -215,7 +212,6 @@ def __init__(self,
                  name,
                  required=True,
                  null=True,
-                 blank=True,
                  default=None,
                  fix_value_fn=None,
                  rename=None,
@@ -227,7 +223,7 @@ def __init__(self,
         super().__init__(name,
                          required=required,
                          null=null,
-                         blank=blank,
+                         blank=True,  # Ignores this column - only null=T/F matters
                          default=default,
                          fix_value_fn=fix_value_fn,
                          rename=rename,
@@ -289,7 +285,6 @@ def __init__(self,
                  name,
                  required=True,
                  null=True,
-                 blank=True,
                  default=None,
                  fix_value_fn=None,
                  rename=None,
@@ -303,7 +298,7 @@ def __init__(self,
         super().__init__(name,
                          required=required,
                          null=null,
-                         blank=blank,
+                         blank=True,  # Ignores this param - only null=T/F matters
                          default=default,
                          fix_value_fn=fix_value_fn,
                          rename=rename,

tests/test_columns.py

@@ -10,7 +10,7 @@
 
 # Constructor tests
 def test_null_forbidden_but_null_default():
-    with pytest.raises(Exception):
+    with pytest.raises(PhaserError):
         Column(name='bogus', null=False, default='homer')
 
 
@@ -20,6 +20,12 @@ def test_invalid_on_error():
     assert "Supported on_error values" in excinfo.value.message
 
 
+@pytest.mark.parametrize('column_type', [IntColumn, FloatColumn, DateColumn, DateTimeColumn, BooleanColumn])
+def test_non_string_column_types_dont_have_empty_param(column_type):
+    with pytest.raises(TypeError):
+        column_type(name='inconsistent', null=False, blank=True)
+
+
 # Simple feature tests
 
 def test_required_values():
@@ -35,6 +41,13 @@ def test_null_forbidden():
         col.check_and_cast_value({'employeeid': None})
 
 
+@pytest.mark.parametrize('value', ["", " ", "\t"])
+def test_blank_forbidden(value):
+    col = Column(name='not_blank', null=False, blank=False)
+    with pytest.raises(DataErrorException):
+        col.check_and_cast_value({'not_blank': value})
+
+
 def test_default_value():
     col = Column('location', default='HQ')
     input = [{'location': 'Atlanta'}, {'location': None}]
@@ -85,7 +98,7 @@ def test_multiple_functions():
     assert col.fix_value("  ACTIVE  ") == "Active  "
 
 
-# Test naming fetaures
+# Test naming features
 
 
 def test_rename():