Fix #1040: diff dim argument should be optional #1131
Conversation
* {Dataset,DataArray}.diff dim argument defaults to last dimension
* add test cases
* add changelog
| @@ -2156,6 +2157,10 @@ def diff(self, dim, n=1, label='upper'): | |||
| raise ValueError('order `n` must be non-negative but got {0}' | |||
| ''.format(n)) | |||
|
|
|||
| # get default last dim if not specified | |||
| if dim is None: | |||
| dim = self.dims[-1] | |||
There was a problem hiding this comment.
This isn't going to work -- self.dims is an sorted dict on Dataset. Moreover, Dataset dims are actually always in sorted order, so the default isn't very sensible.
We could make the default dim be determined per variable, but that wouldn't work very well if different variables had different dimensions or dimension orders.
I think the simplest thing to do is to only allow omitting dim if the dataset has only one dimension.
There was a problem hiding this comment.
Doing this only for 1D is possible, but would make the docstring somewhat less obvious.
Something like dim, optional for 1D ?
|
In reviewing this PR, I'm not really a fan of special casing the 1-D array in the way described above. These sorts of decisions lead to unexpected/confusing behavior when scaling up analysis. Obviously there is a tradeoff here as we diverge from how the |
|
Closing as stale. Please feel free to reopen! |
{Dataset,DataArray}.diff dim argument defaults to last dimension
add test cases
add changelog