Repository: di/vladiate

Issue:
NotEmptyValidator minimal logging
NotEmptyValidator always returns an empty set for it's bad property regardless of failure causing the Vlad class to return False and log the general "Failed :(" message without an indication of the field that failed or the times it failed. Solution proposed below avoids repetition of empty strings in logger output while providing logging on par with the other Validators.

```python
class NotEmptyValidator(Validator):
    ''' Validates that a field is not empty '''

    def __init__(self):
        self.fail_count = 0
        self.empty = set([])

    def validate(self, field, row={}):
        if field == '':
            self.empty.add(field)
            raise ValidationException("Row has empty field in column")

    @property
    def bad(self):
        return self.empty
````


Discussion:

Patch:
diff --git a/setup.py b/setup.py
index 8de8422..62b35f8 100644
--- a/setup.py
+++ b/setup.py
@@ -4,7 +4,7 @@ import sys
 from setuptools import setup, find_packages
 from setuptools.command.test import test as TestCommand
 
-__version__ = '0.0.16'
+__version__ = '0.0.17'
 
 
 class PyTest(TestCommand):
diff --git a/tox.ini b/tox.ini
index 4b85cd7..80f6311 100644
--- a/tox.ini
+++ b/tox.ini
@@ -4,13 +4,29 @@
 # and then run "tox" from this directory.
 
 [tox]
-envlist = py27, py35
+envlist = begin,py{27,35,36},lint,end
+
+[testenv:py27]
+basepython = python2.7
+
+[testenv:py35]
+basepython = python3.5
+
+[testenv:py36]
+basepython = python3.6
+
+[testenv:lint]
+commands = flake8
 
 [testenv]
-commands = coverage erase
-           coverage run --source=vladiate setup.py test
-           coverage combine
-           coverage report --include=*
+commands = coverage run --source=vladiate setup.py test
 deps =
     pytest
     coverage
+    flake8
+
+[testenv:begin]
+commands = coverage erase
+
+[testenv:end]
+commands = coverage report -m --include=*
diff --git a/vladiate/validators.py b/vladiate/validators.py
index bbbb18c..a425c2c 100644
--- a/vladiate/validators.py
+++ b/vladiate/validators.py
@@ -185,16 +185,16 @@ class NotEmptyValidator(Validator):
 
     def __init__(self):
         self.fail_count = 0
+        self.failed = False
 
     def validate(self, field, row={}):
         if field == '':
+            self.failed = True
             raise ValidationException("Row has emtpy field in column")
 
     @property
     def bad(self):
-        # Return an empty set to conform to the protocol. The 'bad' fields
-        # would all be empty strings anyways
-        return set()
+        return self.failed
 
 
 class Ignore(Validator):
diff --git a/vladiate/vlad.py b/vladiate/vlad.py
index b0eb438..0233152 100644
--- a/vladiate/vlad.py
+++ b/vladiate/vlad.py
@@ -41,14 +41,24 @@ class Vlad(object):
                         "  {} failed {} time(s) ({:.1%}) on field: '{}'".format(
                             validator.__class__.__name__, validator.fail_count,
                             validator.fail_count / self.line_count, field_name))
-                    invalid = list(validator.bad)
-                    shown = ["'{}'".format(field) for field in invalid[:99]]
-                    hidden = ["'{}'".format(field) for field in invalid[99:]]
-                    self.logger.error(
-                        "    Invalid fields: [{}]".format(", ".join(shown)))
-                    if hidden:
+                    try:
+                        # If self.bad is iterable, it contains the fields which
+                        # caused it to fail
+                        invalid = list(validator.bad)
+                        shown = [
+                            "'{}'".format(field) for field in invalid[:99]
+                        ]
+                        hidden = [
+                            "'{}'".format(field)
+                            for field in invalid[99:]
+                        ]
                         self.logger.error(
-                            "    ({} more suppressed)".format(len(hidden)))
+                            "    Invalid fields: [{}]".format(", ".join(shown)))
+                        if hidden:
+                            self.logger.error(
+                                "    ({} more suppressed)".format(len(hidden)))
+                    except TypeError:
+                        pass
 
     def _log_missing_validators(self):
         self.logger.error("  Missing validators for:")


Test patch:
diff --git a/vladiate/test/test_validators.py b/vladiate/test/test_validators.py
index 66d431d..7b80686 100644
--- a/vladiate/test/test_validators.py
+++ b/vladiate/test/test_validators.py
@@ -196,7 +196,7 @@ def test_non_empty_validator_fails():
     with pytest.raises(ValidationException):
         validator.validate("")
 
-    assert validator.bad == set()
+    assert validator.bad is True
 
 
 def test_ignore_validator():
diff --git a/vladiate/test/test_vlads.py b/vladiate/test/test_vlads.py
index fd55f94..4b077ae 100644
--- a/vladiate/test/test_vlads.py
+++ b/vladiate/test/test_vlads.py
@@ -3,7 +3,8 @@ import pytest
 from ..vlad import Vlad
 from ..inputs import LocalFile, String
 from ..validators import (
-    EmptyValidator, FloatValidator, SetValidator, UniqueValidator
+    EmptyValidator, FloatValidator, NotEmptyValidator, SetValidator,
+    UniqueValidator,
 )
 
 
@@ -144,3 +145,21 @@ def test_ignore_missing_validators():
 
     assert vlad.validate()
     assert vlad.missing_validators == {'Column B'}
+
+
+def test_when_bad_is_non_iterable():
+    source = String('Column A,Column B\n,foo')
+
+    class TestVlad(Vlad):
+        validators = {
+            'Column A': [NotEmptyValidator()],
+            'Column B': [NotEmptyValidator()],
+        }
+
+    vlad = TestVlad(source=source)
+
+    assert not vlad.validate()
+    assert vlad.validators['Column A'][0].fail_count == 1
+    assert vlad.validators['Column A'][0].bad
+    assert vlad.validators['Column B'][0].fail_count == 0
+    assert not vlad.validators['Column B'][0].bad