fix check dims for ds with multiple vars #761
Conversation
| def _check_dims(da: xr.DataArray, dims=dims) -> bool: | ||
| extra_dims = [dim for dim in da.dims if dim not in dims] | ||
| if len(extra_dims) == 1: | ||
| dims = tuple(extra_dims) + dims | ||
| elif len(extra_dims) > 1: | ||
| raise ValueError("Only 2D and 3D data arrays supported.") | ||
| return da.dims == dims, extra_dims |
There was a problem hiding this comment.
Please don't use closures if they are not necessary, they can cause very unexpected behavour. Could you also give this function a more descriptive name that includes what it actually checks?
There was a problem hiding this comment.
Also your type hint is incorrect, it returns a bool and I think a list of dimentions?
There was a problem hiding this comment.
good suggestion
|
|
||
| extra_dims = [] | ||
| if isinstance(self._obj, xr.DataArray): | ||
| check, dim0 = _check_dims(self._obj) |
There was a problem hiding this comment.
I think this name is very deceptive, since it's not just one dimention but a list of them.
There was a problem hiding this comment.
will change the name
| extra_dims = [] | ||
| if isinstance(self._obj, xr.DataArray): | ||
| check, dim0 = _check_dims(self._obj) | ||
| extra_dims.extend(dim0) |
There was a problem hiding this comment.
Do we want to extend here even if we raise a value error on the next line?
There was a problem hiding this comment.
Only when no error is raised we actually use extra_dims, so I don't think this matters
| if not all([dim in self._obj[var].dims for dim in dims]): | ||
| continue # only check data variables with x and y dims |
There was a problem hiding this comment.
Does this check data vars that have x and y dims or that only have x and y dims?
There was a problem hiding this comment.
That check happens when getting the x and y dims in the raster.x_dim and raster.y_dim properties.
This reverts commit 35e32f1.
Issue addressed
Fixes #760
Explanation
Relax assumption that all dataset variables must have the same dimensions.
Checklist
mainAdditional Notes (optional)
Add any additional notes or information that may be helpful.