-
Notifications
You must be signed in to change notification settings - Fork 11
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
Retain original data's mask when yearly averaging #196
Conversation
I'm not sure what to make of these failures in the Travis 2.7-min environment. Not related to the current PR but also not the same as the ongoing issues c.f. #75 The AppVeyor failures are for python 3.5 and are similar-but-different from either those above or the EmptyHeader of #75. @spencerkclark any ideas on these? Plus a quick review of the PR when you get a chance please :) |
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.
Thanks for taking care of this @spencerahill! Just a couple minor comments, but otherwise this looks good to me.
aospy/test/test_utils_times.py
Outdated
yrs_coord = [2000, 2001, 2004] | ||
yr_avgs = np.array([yr2000, yr2001, yr2004]) | ||
desired = xr.DataArray(yr_avgs, dims=['year'], coords={'year': yrs_coord}) | ||
assert np.allclose(actual.values, desired.values) |
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.
Both here and in the masked test, could we use xr.testing.assert_identical(actual, desired)
? I think this would check the array values and year coordinate all in one line.
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 call; forgot about xr.testing
. I'll use xr.testing.assert_allclose
though; in some cases I was getting failures because my by-hand averaging weren't bitwise identical with the xarray/numpy methods.
docs/whats-new.rst
Outdated
@@ -72,6 +72,9 @@ Bug Fixes | |||
datapoints were invalid instead of just masking points outside the desired | |||
region (fixes :issue:`190` via :pull:`192`). By | |||
`Spencer Clark <https://github.com/spencerkclark>`_. | |||
- Retain original input data's mask during gridpoint-by-gridpoint | |||
tempral averages (fixes :issue:`193` via :pull:`196`). By `Spencer |
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.
Typo: "tempral" -> "temporal"
It seems they've encountered similar error messages in xarray and it's been suggested this is related to a recent update to
I want to say these are still related to #75, but I'm not 100% sure. |
Thanks @spencerkclark. Will merge pending the tests (besides those that #197 will hopefully fix) |
Closes #193.
I still need a test for the actual masking issue; the current test checks un-masked data.
Re-factored a bit in the process, namely moved the function from
Calc
intoutils.times
.