Uh oh!
There was an error while loading. Please reload this page.
fixes for warnings related to unit tests and nan comparisons - #1657
Conversation
jhamman
commented
Oct 25, 2017
Down to 96 (was 372) warnings in my py36 test environment. Two changes that I could uses some input on: 1 - Numpy element wise comparisondefassertEqual(self, a1, a2):
>asserta1==a2or (a1!=a1anda2!=a2)
EDeprecationWarning: elementwise==comparisonfailed; thiswillraiseanerrorinthefuture.@shoyer - you and I put together the assertEqual method a while back. Do we need to check the size of arrays before comparing the values? 2 - Numpy invalid value comparisone.g.: RuntimeWarning: invalidvalueencounteredingreater_equalI'm hoping there is a succinct way of wrapping many of our nan comparing functions in |
jhamman
commented
Oct 27, 2017
shoyer
commented
Oct 27, 2017
Yes, absolutely. Let's do it in stages. |
jhamman
commented
Oct 27, 2017
Okay, so this knocks off the lowest hanging fruit. I'll pull the pytest error out of the travis build and we'll move forward with this. |
shoyer
left a comment
There was a problem hiding this comment.
Oh my... I forgot that assertRaisesRegexp was deprecated!
| variable = (f(self.variable, other_variable) | ||
| if not reflexive | ||
| else f(other_variable, self.variable)) | ||
| with np.errstate(all='ignore'): |
There was a problem hiding this comment.
I don't think we need this here -- this should be caught at the level of the Variable wrapper.
| other_variable = getattr(other, 'variable', other) | ||
| with self.coords._merge_inplace(other_coords): | ||
| f(self.variable, other_variable) | ||
| with np.errstate(all='ignore'): |
shoyer
commented
Oct 27, 2017
@jhamman sorry, looks like you'll need to merge in master, too -- I just put in some conflicting changes with my unicode to netCDF3 PR. |
jhamman
commented
Oct 27, 2017
@shoyer - updated. If all the tests pass, I'll merge tonight. |
shoyer
commented
Oct 28, 2017
This looks good to me. Can you add a note to "what's new" (under bug fixes) about removing the warning when comparing with NaN? |
| with self.assertRaisesRegexp(TypeError, 'pip install netcdf4'): | ||
| with raises_regex(TypeError, 'pip install netcdf4'): | ||
| open_dataset(tmp_file, engine='scipy') |
There was a problem hiding this comment.
@shoyer - any ideas why this test is failing on Appveyor Py3.6: https://ci.appveyor.com/project/shoyer/xray/build/1.0.2844/job/uarr5gsny3wpx9qm#L3326
There was a problem hiding this comment.
Well, we aren't closing this file properly, so I'm not too surprised that Windows is complaining. I don't know why it's only showing up now, though.
For now, just add allow_cleanup_failure=True to the create_tmp_file() call. We'll tackle this later in #1668.
| with open_example_dataset('example_1.nc.gz') as expected: | ||
| with open_example_dataset('example_1.nc') as actual: | ||
| self.assertDatasetIdentical(expected, actual) | ||
| if sys.version_info[:2] < (2, 7): |
| with self.assertRaisesRegexp(TypeError, 'pip install netcdf4'): | ||
| with raises_regex(TypeError, 'pip install netcdf4'): | ||
| open_dataset(tmp_file, engine='scipy') |
There was a problem hiding this comment.
Well, we aren't closing this file properly, so I'm not too surprised that Windows is complaining. I don't know why it's only showing up now, though.
For now, just add allow_cleanup_failure=True to the create_tmp_file() call. We'll tackle this later in #1668.
jhamman
commented
Oct 29, 2017
@shoyer - I think this is all good now. We seem to have acquired another unrelated build failures though... |
shoyer
commented
Oct 29, 2017
Everything passed after the pandas 0.21 fixes in #1669 |
git diff upstream/master | flake8 --diffwhats-new.rstfor all changes andapi.rstfor new APIThis first commit just includes the fixes for the warnings issued in our unit tests. I've temporarily added
-W errorto the travis builds so we can see the warnings/errors for all environments. Next step is to address the warnings in the xarray code itself.