search

jpmckinney / validictory

🎓 deprecated general purpose python data validator
Other
240 stars 57 forks source link

default arguments #50

Closed dmr closed 11 years ago

dmr commented 11 years ago

Follow-up to #27

I started to include @johnnoone patch but I have a questions regarding this:

  1. I removed the test_item ('items': [{'type': 'string'}, {'default': 'baz'},]) case because I think this is too complicated and I don't find it in the specification.
  2. In my current implementation it is possible to have values in "default" that are not correct regarding "type" or more details. How can I include a schema validation in a clean way?

The specification states that the default keyword can contain anything but should (obviously) be valid with the schema. (http://json-schema.org/latest/json-schema-validation.html#anchor101)

My patch is too simple for so much noise but anyway: 54cf528fedb954fadae88fa89a315c86d986138b provides a few tests which describe the new property, I left the test_item tests commented out and in the tests there are my ideas about how this feature should work before merging.

What is your opinion on this?

jamesturk commented 11 years ago

I'm still not 100% comfortable with a validator altering the data, but this is a reasonable approach for the most part.

Before accepting this I'd want to see the default value getting added in and validated though.

dmr commented 11 years ago

I updated my original commit and now the value of a default argument gets validated properly.

One more idea: This feature would be really useful if I could pass a function to it:

{'date': {'default': timezone.now}}

How do you feel about integrating this?

dmr commented 11 years ago

The implementation I provided runs on python2 and python3 and raises a SchemaError if the default property value is not valid.

How do you feel about merging this?

jamesturk commented 11 years ago

Merged this but now I'm having an issue where the python 3 version is failing intermittently.. are you seeing this too?

On Sat, Sep 28, 2013 at 1:25 PM, Daniel Rech notifications@github.comwrote:

The implementation I provided runs on python2 and python3 and raises a SchemaError if the default property value is not valid.

How do you feel about merging this?

— Reply to this email directly or view it on GitHubhttps://github.com/sunlightlabs/validictory/pull/50#issuecomment-25302795 .

jamesturk commented 11 years ago

The issue was that the defaults can be already applied by the time that the rest is validated.. I think this is OK since validation is per-field.

I've removed this test for now as it only works if dict ordering is stable (hence intermittent failure on 3.3)

On Mon, Sep 30, 2013 at 11:47 AM, James Turk jturk@sunlightfoundation.comwrote:

Merged this but now I'm having an issue where the python 3 version is failing intermittently.. are you seeing this too?

On Sat, Sep 28, 2013 at 1:25 PM, Daniel Rech notifications@github.comwrote:

The implementation I provided runs on python2 and python3 and raises a SchemaError if the default property value is not valid.

How do you feel about merging this?

— Reply to this email directly or view it on GitHubhttps://github.com/sunlightlabs/validictory/pull/50#issuecomment-25302795 .

dmr commented 11 years ago

Thanks for merging :)

On my computer (OSX 10.7.5) the test_property_default_denied_does_not_change_original_data_on_error passes with python 3.3.2, sadly I cannot reproduce the error you describe.

I think this test is still relevant but maybe we can only include only one property to prevent the order issue?

dmr commented 11 years ago

Was this the reason you removed python3.2?