541 oc.select #687
Merged
541 oc.select #687
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omry
requested review from
odelalleau and
Jasha10
and removed request for
odelalleau
odelalleau
requested changes
Apr 17, 2021
omegaconf/resolvers/oc/__init__.py
Outdated
Comment on lines
97
to
108
| def select( | ||
| key: str, | ||
| default: Any = _DEFAULT_SELECT_MARKER_, | ||
| *, | ||
| _parent_: Container, | ||
| ) -> Any: | ||
| from omegaconf._impl import select_value | ||
|
|
||
| if default is _DEFAULT_SELECT_MARKER_: | ||
| return select_value(cfg=_parent_, key=key, absolute_key=True) | ||
| else: | ||
| return select_value(cfg=_parent_, key=key, absolute_key=True, default=default) |
There was a problem hiding this comment.
Why use a different marker?
Suggested change
| def select( | |
| key: str, | |
| default: Any = _DEFAULT_SELECT_MARKER_, | |
| *, | |
| _parent_: Container, | |
| ) -> Any: | |
| from omegaconf._impl import select_value | |
| if default is _DEFAULT_SELECT_MARKER_: | |
| return select_value(cfg=_parent_, key=key, absolute_key=True) | |
| else: | |
| return select_value(cfg=_parent_, key=key, absolute_key=True, default=default) | |
| def select( | |
| key: str, | |
| default: Any = _DEFAULT_MARKER_, | |
| *, | |
| _parent_: Container, | |
| ) -> Any: | |
| from omegaconf._impl import select_value | |
| return select_value(cfg=_parent_, key=key, absolute_key=True, default=default) |
There was a problem hiding this comment.
good point. Initially I was passing it into select_value.
There was a problem hiding this comment.
I think it can be simplified further (see my suggestion: no more if)
There was a problem hiding this comment.
yeah, for some reason I thought using the same marker object select_value is using would cause issues.
odelalleau
reviewed
Apr 17, 2021
Jasha10
requested changes
Apr 17, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #541