issue_comments: 338446509

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/issues/884#issuecomment-338446509 https://api.github.com/repos/pydata/xarray/issues/884 338446509 MDEyOklzc3VlQ29tbWVudDMzODQ0NjUwOQ== 1217238 2017-10-22T02:36:39Z 2017-10-22T02:36:39Z MEMBER

Another question is what to do with x in ds (i.e.,Dataset.__contains__). Currently it checks data variables and coordinates, but not dimensions (as @fujiisoup pointed out in https://github.com/pydata/xarray/pull/1632#issuecomment-336637245), which already feels somewhat inconsistent.

If we were starting over, I might change how __contains__ works for Dataset to only include data variables, but I don't think it's worth it at this time:

  • Unlike the current version of __iter__, __contains__ is actually useful in its current state and I suspect is widely used. Issuing a deprecation warning for 'variable' in ds would annoy lots of users, for no particularly good reason. In many cases (checking data variables), the behavior would not actually change.
  • There is also the expectation that k in ds is equivalent to ds[k] for mapping types, which is currently mostly true for xarray.Dataset.
  • The main advantage I see to changing the behavior of k in ds is that it would remove most of the remaining use cases for Dataset.data_vars, which would let us eventually remove data_vars. But given that ds[k] for coordinates k will continue to be supported, I think I would still recommend explicitly writing k in ds.data_vars for cases where the data/coordinates distinction matters.

Related: over in https://github.com/pydata/xarray/pull/1645, I am deprecating the current behavior of DataArray.__contains__, so that we can make it check array values instead of DataArray.coords in the future.

If we aren't going to fundamentally change the behavior of k in ds to exclude coordinates, then we should probably update it, at @fujiisoup suggests, to also include MultiIndex levels and dimension names (i.e., the stuff checked in xarray.core.dataset._get_virtual_variable()).

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   160505403