-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Validate output dimension sizes with apply_ufunc #2155
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
Validate output dimension sizes with apply_ufunc #2155
Conversation
Fixes GH1931 Uses of apply_ufunc that change dimension size now raise an explicit error, e.g., >>> xr.apply_ufunc(lambda x: x[:5], xr.Variable('x', np.arange(10))) ValueError: size of dimension 'x' on inputs was unexpectedly changed by applied function from 10 to 5. Only dimensions specified in ``exclude_dims`` with xarray.apply_ufunc are allowed to change size.
xarray/core/computation.py
Outdated
for dim, new_size in var.sizes.items(): | ||
if dim in dim_sizes and new_size != dim_sizes[dim]: | ||
raise ValueError( | ||
'size of dimension {!r} on inputs was unexpectedly changed ' |
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.
E501 line too long (80 > 79 characters)
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.
One comment, but it looks good to me.
xarray/core/computation.py
Outdated
result_data = [result_data] | ||
|
||
output = [] | ||
for dims, data in zip(output_dims, result_data): |
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.
I think it would be nice if we raise an informative error message when the number of output_dims
and result_data
is different.
Currently this will silently drop some items if they have a different number of items.
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.
Good point -- done!
I plan to merge this shortly, unless anyone else has feedback here |
Fixes #1931
Uses of apply_ufunc that change dimension size now raise an explicit error,
e.g.,
whats-new.rst
for all changes andapi.rst
for new API (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later)