Skip to content

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

Merged
merged 5 commits into from
May 31, 2018

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented May 17, 2018

Fixes #1931

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.
  • Closes apply_ufunc produces illegal coordinate sizes #1931 (remove if there is no corresponding issue, which should only be the case for minor changes)
  • Tests added (for all bug fixes or enhancements)
  • Tests passed (for all non-documentation changes)
  • Fully documented, including whats-new.rst for all changes and api.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)

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.
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 '
Copy link
Contributor

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)

Copy link
Member

@fujiisoup fujiisoup left a 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.

result_data = [result_data]

output = []
for dims, data in zip(output_dims, result_data):
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point -- done!

@shoyer shoyer mentioned this pull request May 25, 2018
8 tasks
@shoyer
Copy link
Member Author

shoyer commented May 29, 2018

I plan to merge this shortly, unless anyone else has feedback here

@shoyer shoyer merged commit 0e21fdf into pydata:master May 31, 2018
@shoyer shoyer deleted the apply_ufunc_output_dim_validation branch May 31, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apply_ufunc produces illegal coordinate sizes
3 participants