Implicit default value for optional key in mappings? about strictyaml HOT 16 CLOSED

Implemented in version 0.15.0. Let me know if you see any issues.

from strictyaml.

Yeah, this sounds like a good idea. Something like this you mean?

Map({Optional("optional"): Str(), "mandatory": Str()}, defaults={"optional": "mydefault"})

I have the same kind of problem with some of my apps, although I usually fill in the default by going yaml.data.get("optional", "thedefaultiwanted"). When looking at https://github.com/gjcarneiro/yacron/blob/master/yacron/config.py by @gjcarneiro I figured it might be better for coherence if he could define defaults alongside the schema instead of in a different structure.

from strictyaml.

Sorry that took so long. It's fixed now in 0.15.1 and your tests now seem to be passing.

from strictyaml.

I don't know if and in case how this is done elsewhere. Would be good to be consistent. In my opinion putting default values alongside the schema seems suitable as well. It could be reasonable to put the definition of the default value closer to the definition of the key e.g. something like Map({Optional(key="optional", default="mydefault"): Str(), "mandatory": Str()}).

from strictyaml.

Yeah, maybe that would be better actually. It would be closer to the validator and it would naturally prevent putting in default keys for keys that don't actually exist.

A few other things to bear in mind:

• The default, whatever it is, should be validated against the schema and raise an exception if it's not of the correct type. I think catching YAMLSerializationError and raising a different exception (YAMLSchemaException perhaps) should work.
• I'm not sure if turning data in to YAML should respect the defaults. i.e. should we get:

as_document({"nonoptional": "x", "optional": "mydefault"}, yourschema).as_yaml() == "nonptional: x"

Or:

as_document({"nonoptional": "x", "optional": "mydefault"}, yourschema).as_yaml() == "nonptional: x\nmydefault: mydefault"

from strictyaml.

• I'm not sure if turning data in to YAML should respect the defaults. i.e. should we get:

Me neither. Speaking with my use case in mind I'd consider default values in generated YAML "noise" because I am biased and know my schema and the default values will hardly change. Having them in the YAML wouldn't require a user to know the schema and the documentation (which could be inconsistent with the schema).

from strictyaml.

Ok so option 1? I was leaning towards that myself too.

Btw, you seemed keen to do a PR yesterday. Would you prefer it if:

• I write this.
• I write the spec/tests on a new branch, you do a PR with the code changes.
• You write this.

I'm ok with any of these.

from strictyaml.

It's not blocking me and it's minor for me right now. We can keep this open until first of us both is blocked.

from strictyaml.

Docs : https://hitchdev.com/strictyaml/using/alpha/compound/optional-keys-with-defaults/

from strictyaml.

An implication w.r.t. schema design:

Default values are only supported with load( and not with revalidate(, right? From a logical point of view this makes total sense but implies limitations w.r.t. separation of validation logic for big schemas. I usually validate big schemas in sub-sequence logic: validate overall schema as part of load(), revalidate parts of the overall schema with separate revalidate( steps. In case one want's to use default values for optional keys the corresponding part of the schema needs to be validated as part of load load(. E.g. for deeply nested optional keys with default values in the lowest layer one is forced to validate the overall schema with a single step using load(.

from strictyaml.

It should work. I just tested:

loose_schema = MapPattern(Str(), Str())
strict_schema = Map({"a": Int(), Optional("b", default=False): Bool(), })
myyaml = load(yaml_snippet, loose_schema)
myyaml.revalidate(strict_schema)
assert myyaml.data == OrderedDict([('b', False), ('a', 1)])

Or did you mean another kind of revalidate scenario?

FWIW I've been exploratory testing it but I'm still not 100% convinced this feature is totally bug free, so if you do see any odd behavior please do let me know.

from strictyaml.

Or did you mean another kind of revalidate scenario?

I have two branches config-defaults-revalidate and config-defaults. In the first one I added default values with my current re-validation logic (tests fail). In the second branch I added default values and refactored back to single validation logic with load( (tests fail).

I don't know yet why tests fail in both cases. Tests can easily be run locally (pipenv run tox -e pytest is sufficient) like described in contribution docs.

FWIW I've been exploratory testing it but I'm still not 100% convinced this feature is totally bug free, so if you do see any odd behavior please do let me know.

No worries. Is 100% confidence into tests realistic? 😄

from strictyaml.

Cool! I'll check it out and figure out why it's failing.

from strictyaml.

Thx a lot.

from strictyaml.

Yep, this is a bug. Thanks for letting me know!

I will try and roll out a fix later today.

from strictyaml.

Thanks both for implementing this most useful feature.

from strictyaml.