-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add choose_any
and choose_all
methods
#3
Conversation
andersy005
commented
Aug 13, 2021
- Choose ANY
- Choose ALL
Note that with |
I think this is perfect. We can add |
xcollection/main.py
Outdated
@@ -79,3 +80,31 @@ def values(self) -> typing.Iterable[xr.Dataset]: | |||
|
|||
def items(self) -> typing.Iterable[typing.Tuple[str, xr.Dataset]]: | |||
return self.datasets.items() | |||
|
|||
def choose_all(self, data_vars: typing.Union[str, typing.List[str]]) -> 'Collection': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we do
def choose(self, data_vars, *, mode="all")
and mode
could be "any"
.
We should add an equivalent to Dataset.filter_by_attrs
but with a different name potentially to allow users to filter by standard_name
etc. iris
may also have some related functionality that we could copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the default be mode="any"
and then the user needs to explicitly specify when they expect the variable to be in every dataset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the default be
mode="any"
and then the user needs to explicitly specify when they expect the variable to be in every dataset?
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andersy005 I think this looks good. Thanks for putting this together.