Support None option values. #20736
Support None option values. #20736
Conversation
The python options parser allows default=None, so we must support it in the rust parser. A None value can currently only be obtained via a default of None: if a default is not None then the option will have that value if it doesn't take a non-None value from some other source. We don't currently support setting a value to None in args/env/config in Python, so we don't support it here either. However the data structures could support this if we want to in the future.
| pub struct OptionalOptionValue<T> { | ||
| pub derivation: Option<Vec<(Source, T)>>, | ||
| pub source: Source, | ||
| pub value: Option<T>, |
There was a problem hiding this comment.
It looks like this is almost identical to OptionValue<Option<T>> (except derivation becomes Option<Vec<(Source, Option<T>)>); what do you think about using that instead?
I think we can still add this convenience method, as I believe this should work:
impl<T> OptionValue<Option<T>> {
fn unwrap(self) -> OptionValue<T> { ... }
}Alternatively, add combinator methods like fn map<U>(self, f: FnOnce(T) -> U) -> OptionValue<U> to OptionValue<T>, then the places that need unwrap can write some_option_value.map(|v| v.unwrap()) or some_option_value.map(Option::unwrap).
There was a problem hiding this comment.
We could. That type for a derivation would be substantial overkill, since only defaults can be None. But I'm not sure having both types is a big problem? They are lightweight containers with no associated logic.
There was a problem hiding this comment.
Thanks for the review. Will merge as-is for now, and consider simplifying this once the Python bindings are done and we can see how it all fits together.
There was a problem hiding this comment.
Yep, not a problem at all, just exploring the solution space. Happy as is!
The python options parser allows default=None,
so we must support it in the rust parser.
A None value can currently only be obtained
via a default of None: if a default is not None
then the option will have that value if it doesn't
take a non-None value from some other source.
We don't currently support setting a value to None in
args/env/config in Python, so we don't support
it here either. However the data structures could
support this if we want to in the future.