home / github / issue_comments

Menu
  • GraphQL API
  • Search all tables

issue_comments: 588165645

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/3774#issuecomment-588165645 https://api.github.com/repos/pydata/xarray/issues/3774 588165645 MDEyOklzc3VlQ29tbWVudDU4ODE2NTY0NQ== 35968931 2020-02-19T11:07:26Z 2020-02-19T11:34:41Z MEMBER

Hi @kazimuth , thanks for highlighting this!

This is a case which I can see the argument for handling, but we apparently didn't think of when implementing combine_by_coords.

What this really boils down to is "should combine_by_coords combine by 0D non-dimension coordinates as well as 1D dimension coordinates?". I think yes, that makes sense.

On the other hand, this could be seen as "magic"

This function is supposed to be "magic". But the behaviour still needs to be clearly-defined.

maybe the code that concatenates dimensions should look for full dimensions first, and point coordinates second

Yes - I actually got your example to work with just a few extra lines. Change the top of _infer_concat_order_from_coords to read like this:

```python def _infer_concat_order_from_coords(datasets):

# All datasets have same variables because they've been grouped as such
ds0 = datasets[0]

concat_dim_candidates = list(ds0.dims.keys())

# Look for 0D coordinates which can be concatenated over but aren't dims
non_dimension_0d_coords = [coord for coord in ds0.coords
                           if coord not in ds0.dims
                           and len(ds0[coord].dims) == 0]
for coord in non_dimension_0d_coords:
    if all(coord in ds.coords for ds in datasets):
        datasets = [ds.expand_dims(coord) for ds in datasets]
        concat_dim_candidates.append(coord)

concat_dims = []
tile_ids = [() for ds in datasets]

for dim in concat_dim_candidates:
    ...   # The rest of the function is unchanged

```

it might also be a breaking change to combine_by_coords; I'm not sure.

I don't think it would be breaking... the only cases which would behave differently are ones which would previously would have thrown an error. Would have to think about this though.

Might you be interested in submitting a PR along these lines @kazimuth ? It would require some more robust input checks, some tests for different cases, and some minor changes to docstrings I think.

{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  566490806
Powered by Datasette · Queries took 0.815ms · About: xarray-datasette