Repository: coverahealth/dataspec

Issue:
Predicates decorated by `pred_to_validator` are not properly converted into Validator Specs
```python
from dataspec import s, pred_to_validator

>>> @pred_to_validator("This value is invalid")
... def is_valid(v) -> bool:
...     return bool(v)
...
>>> spec = s(is_valid)
>>> spec
PredicateSpec(tag='is_valid', _pred=<function is_valid at 0x109a09b70>, _conformer=None)
```

This created workarounds requiring people to import `ValidatorSpec` from `dataspec.base`.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a35155c..26d754a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Add `SpecPredicate` and `tag_maybe` to the public interface (#49)
 
+### Fixed
+- Predicates decorated by `pred_to_validator` will now properly be converted into
+  validator specs (#54)
+
 ## [v0.2.4] - 2019-12-19
 ### Added
 - Add `s.blankable` Spec for specifying optional string values (#45)
diff --git a/README.md b/README.md
index 7ee2125..d59c11a 100644
--- a/README.md
+++ b/README.md
@@ -112,6 +112,44 @@ spec.is_valid("4716df50-0aa0-4b7d-98a4-1f2b2bcb1c6b")  # True
 spec.is_valid("b4e9735a-ee8c-11e9-8708-4c327592fea9")  # False
 ```
 
+### Validator Specs
+
+Simple predicates make fine specs, but are unable to provide more details to the caller
+about exactly why the input value failed to validate. Validator specs directly yield
+`ErrorDetails` objects which can indicate more precisely why the input data is failing
+to validate.
+
+```python
+def _is_positive_int(v: Any) -> Iterable[ErrorDetails]:
+    if not isinstance(v, int):
+        yield ErrorDetails(
+            message="Value must be an integer", pred=_is_positive_int, value=v
+        )
+    elif v < 1:
+        yield ErrorDetails(
+            message="Number must be greater than 0", pred=_is_positive_int, value=v
+        )
+
+spec = s(_is_positive_int)
+spec.is_valid(5)      # True
+spec.is_valid(0.5)    # False
+spec.validate_ex(-1)  # ValidationError(errors=[ErrorDetails(message="Number must be greater than 0", ...)])
+```
+
+Simple predicates can be converted into validator functions using the builtin
+`pred_to_validator` decorator:
+
+```python
+@pred_to_validator("Number must be greater than 0")
+def _is_positive_num(v: Union[int, float]) -> bool:
+    return v > 0
+
+spec = s(_is_positive_num)
+spec.is_valid(5)      # True
+spec.is_valid(0.5)    # True
+spec.validate_ex(-1)  # ValidationError(errors=[ErrorDetails(message="Number must be greater than 0", ...)])
+```
+
 ### UUID Specs
 
 In the previous section, we used a simple predicate to check that a UUID was a certain
@@ -397,6 +435,24 @@ All Specs can be created with optional tags, specified as a string in the first
 positional argument of any spec creation function. Tags are useful for providing
 useful names for specs in debugging and validation messages.
 
+## Patterns
+
+### Factories
+
+Often when validating documents such as a CSV or a JSON blob, you'll find yourself
+writing a series of similar specs again and again. In situations like these, it is
+recommended to create a factory function for generating specs consistently. `dataspec`
+uses this pattern for many of the common spec types described above. This encourages
+reuse of commonly used specs and should help enforce consistency across your domain.
+
+### Reuse
+
+Specs are designed to be immutable, so they may be reused in many different contexts.
+Often, the only the that changes between uses is the tag or conformer. Specs provide a
+convenient API for generating copies of themselves (not modifying the original) which
+update only the relevant attribute. Additionally, Specs can be combined in many useful
+ways to avoid having to redefine common validations repeatedly.
+
 ## License
 
 MIT License
diff --git a/src/dataspec/base.py b/src/dataspec/base.py
index b46fedd..5c9915e 100644
--- a/src/dataspec/base.py
+++ b/src/dataspec/base.py
@@ -677,6 +677,7 @@ def pred_to_validator(
                     value=v,
                 )
 
+        validator.is_validator_fn = True  # type: ignore
         return validator
 
     return to_validator
@@ -731,7 +732,9 @@ def make_spec(  # pylint: disable=inconsistent-return-statements
             # Some builtins may not be inspectable
             sig = None
 
-        if sig is not None and sig.return_annotation is Iterator[ErrorDetails]:
+        if (
+            sig is not None and sig.return_annotation is Iterator[ErrorDetails]
+        ) or getattr(pred, "is_validator_fn", False):
             return ValidatorSpec(
                 tag or pred.__name__, cast(ValidatorFn, pred), conformer=conformer
             )


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index f80a7eb..b5c19a5 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -3,7 +3,7 @@ from typing import Iterator
 import pytest
 
 from dataspec import ErrorDetails, ValidationError, s
-from dataspec.base import PredicateSpec, ValidatorSpec
+from dataspec.base import PredicateSpec, ValidatorSpec, pred_to_validator
 
 
 class TestSpecConstructor:
@@ -34,6 +34,13 @@ class TestSpecConstructor:
 
         assert isinstance(s(is_valid), PredicateSpec)
 
+    def test_pred_to_validator(self):
+        @pred_to_validator("This value is invalid")
+        def is_valid(v) -> bool:
+            return bool(v)
+
+        assert isinstance(s(is_valid), ValidatorSpec)
+
     def test_no_signature_for_builtins(self):
         s.all(s.str(), str.istitle)