Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validator examples: behavior does not match what is shown in comments #90

Open
stefcoetzee opened this issue Sep 14, 2023 · 2 comments
Open

Comments

@stefcoetzee
Copy link

stefcoetzee commented Sep 14, 2023

Steps to reproduce

  1. Install Dataspec in a suitable Python virtual environment;
  2. Import the necessary classes, functions, and variables;
  3. Copy example code from section Predicate and Validators; and
  4. Execute example calls to spec methods.

_is_positive_int

Setup

from collections.abc import Iterable
from typing import Any

from dataspec import ErrorDetails, s


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)

Expected behavior

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", ...)])

Actual behavior

spec.is_valid(5)    # True
spec.is_valid(0.5)  # True
spec.is_valid(-1)   # No return value

_is_positive_num()

Setup

from typing import Union

from dataspec import pred_to_validator, s


@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)

Expected behavior

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", ...)])

Actual behavior

The behavior of _is_positive_num appears to be inverted.

spec.is_valid(5)      # False
spec.is_valid(0.5)    # False
spec.validate_ex(-1)  # No return value
spec.validate_ex(1)   # ValidationError: [ErrorDetails(message="Number must be greater than 0", ...)]

System configuration

Python version: 3.10.11
Dataspec version: 0.3.2


I'm not yet sure why this happens, as the validation function definitions seem correct.

@stefcoetzee stefcoetzee changed the title Predicates and validators examples' does not match what is shown in comments Predicates and validators examples' behavior does not match what is shown in comments Sep 14, 2023
@stefcoetzee stefcoetzee changed the title Predicates and validators examples' behavior does not match what is shown in comments Predicates and validators examples: behavior does not match what is shown in comments Sep 14, 2023
@stefcoetzee stefcoetzee changed the title Predicates and validators examples: behavior does not match what is shown in comments Validator examples: behavior does not match what is shown in comments Sep 14, 2023
@stefcoetzee
Copy link
Author

In the case of _is_positive_num, I believe the validator should use the complement of the predicate. Thus, it should be:

@pred_to_validator("Number must be greater than 0", complement=True)
def _is_positive_num(v: Union[int, float]) -> bool:
  return v > 0

@stefcoetzee
Copy link
Author

stefcoetzee commented Sep 14, 2023

In the case of _is_positive_int, a PredicateSpec is returned instead of a ValidatorSpec. Importing ValidatorSpec and defining spec as:

spec = ValidatorSpec("_is_positive_int", _is_positive_int)

Results in the expected behavior.

From dataspec.base, in function make_spec, a ValidatorSpec is returned when the return annotation is Iterator[ErrorDetails] or the predicate function attribute is_validator_fn is set to True:

# dataspec/base.py

def make_spec(..., *preds: SpecPredicate, ...) -> Spec:

    ...

    try:
        pred = preds[0]
    except IndexError:
        raise TypeError("Expected some spec predicate")

    ...

    elif callable(pred):
        try:
            sig: Optional[inspect.Signature] = inspect.signature(pred)
        except (TypeError, ValueError):
            # Some builtins may not be inspectable
            sig = None

        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
            )
        else:
            return PredicateSpec(
                tag or pred.__name__, cast(PredicateFn, pred), conformer=conformer
            )

    ...

The example return annotation is Iterable[ErrorDetails], which isn't quite what is required.

Thus, _is_positive_int definition should be:

def _is_positive_int(v: Any) -> Iterator[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 defined using the above validator definition yields the desired behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant