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

Support defaults via Optional #52

Merged
merged 6 commits into from
Feb 27, 2015
Merged

Conversation

erikrose
Copy link
Contributor

This lets us insert default values for missing dict keys. I independently arrived at this API before seeing that it's also a leading contender in #19, so it's at least unsurprising for 2 people. :-)

I had to do a minor twiddle of prioritization so that more generic keys like str wouldn't eat the Optionals' lunch, but that shouldn't affect any conceivable non-crashing use case. (See one of the commit messages for details.)

This will let Optional('foo', default=1) take precedence over simply `str` and thus have the opportunity to insert its default.

The only case for which this should change behavior is if somebody was comparing against a type with a `validate` attr. In that case, the logic would have gone down the wrong path anyway, attempting to call validate() on the type itself.
Then we don't have to filter them back out later.
Error trapping on Optional construction is to follow, split out for ease of review, since it will likely require some refactoring.
@erikrose
Copy link
Contributor Author

The reason the defaults are used verbatim, rather than being passed through any value validators, is that it's more convenient in my use cases to come up with a programmatic value than a raw representation. You can always get the opposite behavior by factoring out any validators in question and manually passing a raw value through them. Perhaps more compellingly, inverting the opposite behavior would not always be possible.

@erikrose
Copy link
Contributor Author

I believe this also fixes #23.

@keleshev
Copy link
Owner

Thanks, Erik!

keleshev added a commit that referenced this pull request Feb 27, 2015
@keleshev keleshev merged commit cffb3f3 into keleshev:master Feb 27, 2015
@erikrose
Copy link
Contributor Author

Thanks for the quick merge! :-D

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

Successfully merging this pull request may close these issues.

2 participants