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

Conditional validation? #35

Closed
jdandrea opened this issue Feb 5, 2016 · 7 comments
Closed

Conditional validation? #35

jdandrea opened this issue Feb 5, 2016 · 7 comments
Milestone

Comments

@jdandrea
Copy link

jdandrea commented Feb 5, 2016

Let's say we have this contrived example YAML:

foods:
  first_food:
    classification: fruit
    properties:
      name: apple
      color: red
      other: properties_go_here
  second_food:
    classification: vegetable
    properties:
      name: carrot
      color: orange
      other: properties_go_here

Presume that we can't change the format. In this case, each food gets an arbitrary (key) name, and the classification of each food must be kept separate from its properties.

I'm trying to find a way to conditionally validate the name property of a food based on its classification. In this example, if the classification is "fruit" then I want to validate that food's name property based on a list of fruits. If the classification is "vegetable" then I want to validate that food's name property based on a list of vegetables.

I started to imagine a schema to help with this:

mapping:
  "foods":
    mapping:
      regex;(.+):
        allowempty: True
        mapping:
          "classification": { type: str, pattern: fruit }
          "properties":
            allowempty: True
            mapping:
              "name": { type: str, pattern: (apple|orange|grape) }
      regex;(.+):
        allowempty: True
        mapping: 
          "classification": { type: str, pattern: vegetable }
          "properties":
            allowempty: True
            mapping:
              "name": { type: str, pattern: (carrot|broccoli|cucumber) }

Only this doesn't quite work:

 ERROR - validation.invalid
 ERROR -  --- All found errors ---
 ERROR - [u"Value 'apple' does not match pattern '(carrot|broccoli|cucumber)'. Path: '/foods/first_food/properties/name'", u"Value 'fruit' does not match pattern 'vegetable'. Path: '/foods/first_food/classification'"]

I think I'm trying to say this: "Each food mapping must pass any (not all) of these regex-based validations." I'm not sure if I can do that though. I thought perhaps with YAML anchors, but perhaps not. The next best thing I think I can do is make these separate schemas and run each one in turn.

Thoughts?

(Meanwhile, I'm really enjoying exploring pykwalify!)

@Grokzen
Copy link
Owner

Grokzen commented Feb 7, 2016

Hmm. Conditional validation based on some completely different part of the data tree is nothing that is really supported by either kwalify or pykwalify and i am not really sure how this kind of syntax would even look to support this kind of operation. You would need some kind of logic with if/else/elseif and some way to refer to data inside the data structure and i think it would just be messy to merge together this with the regular kwalify standard.

The only solution that i can think of right now is to use the following feature https://github.com/Grokzen/pykwalify/blob/unstable/docs/Extensions.md and do some magic inside a few functions where you call one function on the classification and one on the properties and then do the validation inside those functions. It is kinda messy, but it might be a way to solve this.

@jdandrea
Copy link
Author

jdandrea commented Feb 8, 2016

Excellent points. Thank you! I tried the extensions, and the capability is quite wonderful. Still, it seemed like overdoing it for this particular use case (like you wrote, messy).

Then I thought: What I really have here are multiple schemas, one for each pass (fruit, vegetable, etc.). Why not just run them one at a time, and-ing the results together?

I don't know if there's any performance benefit by having pykwalify allow multiple schemas—that is, multiple YAML docs—even within a single file (which YAML allows for!). Apart from that, I'm going to create individual schemas and run pykwalify over each of them. Thoughts?

@jdandrea
Copy link
Author

jdandrea commented Feb 8, 2016

Ahh, turns out the individual schemas won't work in this case because of the repeated use of regex;(.+): (each template fails for all the food IDs that don't fit its particular type of food). If I could say "only have to match at least one of these" then I'd be set. Not sure if that's congruent with kwalify.

The thought of putting too many smarts into the extension also takes away from the awesomeness of using (py)kwalify.

@Grokzen
Copy link
Owner

Grokzen commented Dec 21, 2016

Hi @jdandrea. I am going to close down this issue becuase i do not think that anything will come out from it as it stands currently. I have no plans of adding in some kind of if/elif/else code into the schema directly becuase that will just make the language diverge far away from the original kwalify.

My second argument is that really can do all of this conditional validation inside a custom extension/function. Yes it will be very annoying to make a full function for this small case but atleast in there you have all the freedom you want to do whatever you want. You have access to all rule objects (the schema) so you can do what ever you want inside there.

A comment on the "multiple validations on the same key/regex" problem there will not be any solution to that right now at least. I might take a stab at it in the future but right now this feature is not in the scope of being implemented.

@Grokzen Grokzen closed this as completed Dec 21, 2016
@Grokzen
Copy link
Owner

Grokzen commented Dec 21, 2016

Multiple validation paths for mapping is already tracked in issue #45 and it takes presidence over this issues

@jdandrea
Copy link
Author

Hi! I think the case for a custom extension/function makes good sense and prevents things from diverging too far afield. Thanks for the update!

@Grokzen
Copy link
Owner

Grokzen commented Dec 21, 2016

@jdandrea If you dig into extensions more, please just open up a issue if you find anything not working or if there is a need for any other data to be provided into the functions that would help towards solving this problem.

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

No branches or pull requests

2 participants