issue_comments

Done. Not sure why only AppVeyor started :confused:

Assuming we do add the filter method, maybe filter_by_attrs?

Makes sense. I will modify this soon.

I do think the name is a mouthful, though :).

I agree that is a mouthful but then again, I was trying to be consistent with the existing versions. However, I don't really have a strong opinion on the matter and I am fine with whatever you decide. Should I renamed then? filter_attrs? (Sounds weird though b/c you are not filtering the attrs, but the variables based on the attrs :expressionless:)

I fine with whatever you decide but here are my two cents: - get_variables_by_attributes is the same name of this method in netcd4 and some java netcdf libraries. So I'd rather not have a specialized version for attributes than adding it with name with a different name. - I see the elegance in s.filter(lambda x: x.attrs['standard_name'] == 'convective_precipitation_flux') and I like it a lot! But the specialized version for attributes is more compact to write and, at least in my field, filtering by attributes is more common making this version more convenient.

Feel free to close this if you don't think it is worth adding the specialized version. Or let me know if you want to rename it.

Is there still an interested in this or should I close in light of https://github.com/pydata/xarray/issues/883?

Agreed -- this is almost ready. Please add to the API docs (api.rst) and do the docstring fixes.

Not sure if I did this right :grimacing:

Rebased and ready for another round of reviews :wink:

The appveyor build failure looks unrelated to this change -- something about conda dependencies.

I am experiencing that in other projects. It is actually a bad download of miniconda, and powershell makes the error message unusable. Re-starting should fix it. (BTW miniconda is pre-installed on AppVeyor and that download is not needed.)

@jhamman and @shoyer if the tests passes this is ready for another round of review. (Let me know if I should squash the previous ones to make it easier to review.)

@shoyer this is ready for another round of review.

Well, if you guys are mostly excited about using this for coordinate variables, another consistent choice would be to return a list of matching DataArrays. But if we want to return a Dataset, we should only do data variables, because it's weird to lose all the describing coordinates when you match, e.g., standard_name="air_temperature".

We agree with you and I prefer to return a Dataset.

Right now we always go to netCDF4-python to do this low-level CF interpretation stuff. If we start using xarray for that task we should improve xarray, to take advantage of the conventions (CF/SGRID/UGRID), instead of using xarray to just find the coords data. (For example: creating the z coordinates from non-dimension coordinates and add that to the Dataset coords automatically.)

After discussing with my CF guru (@rsignell-usgs) and the original author of the get_variables_by_attributes (@kwilcox) I jumped the fence and I am OK leaving the xarray model pure and filtering only the data variables.

should this filter both data variables and coordinates or only data variables?

I thought a little bit more about this and now I am on the fence. The pros of filtering only data variables are a nice and clean Dataset object, and overall consistency with the high level xarray model. The cons are that we cannot do the filtering on the coords (obviously), but most of the time that we need to do that we go to a lower level object like netCDF4.Dataset. However, it would be nice if both xarray and netCDF4 behaved the same way...

I am 51% with filtering both (and the current implementation does that) but I will leave the final decision to you.

An important design question: should this filter both data variables and coordinates or only data variables? My thought is that it's only worth filtering data variables -- filtering out unmatched coordinates is not very useful.

I understand that returning coordinates without a data variable associated to them seems weird to the high level model of xarray.Dataset , but I disagree that it is not very useful. In fact that is the most common operation I do: find coordinates based on axis, formula_terms, etc and construct common grids for plotting and/or building the z coords from non-dimension coords.