-
Notifications
You must be signed in to change notification settings - Fork 61
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
Implicit default value for optional key in mappings? #45
Comments
Yeah, this sounds like a good idea. Something like this you mean?
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. |
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 |
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:
Or:
|
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). |
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'm ok with any of these. |
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. |
Implemented in version 0.15.0. Let me know if you see any issues. |
An implication w.r.t. schema design: Default values are only supported with |
It should work. I just tested:
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. |
I have two branches I don't know yet why tests fail in both cases. Tests can easily be run locally (
No worries. Is 100% confidence into tests realistic? 😄 |
Cool! I'll check it out and figure out why it's failing. |
Thx a lot. |
Yep, this is a bug. Thanks for letting me know! I will try and roll out a fix later today. |
Sorry that took so long. It's fixed now in 0.15.1 and your tests now seem to be passing. |
Thanks both for implementing this most useful feature. |
According to the optional keys docs and the sources there is no support for an implicit default value for an optional key. My use case: I potentially have to configure a very long list of key value mappings for my command line application. The majority of the
optional-key
s usually have a default value which is implicit but from application/user perspective reasonable, not surprising. I know implicit data may be bad. Anyway... what do you think?The text was updated successfully, but these errors were encountered: