-
Notifications
You must be signed in to change notification settings - Fork 300
PP + merge test fixes #2728
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
PP + merge test fixes #2728
Conversation
| self.assertArrayEqual(result.data, expected) | ||
| self.assertEqual(result.dtype, self.dtype) | ||
| self._check_fill_value(result, fill0, fill1) | ||
| self._check_fill_value(result) |
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.
For tests including ndarray (e.g. test__ndarray_ndarray, test__masked_ndarray, test__ndarray_masked) it seems unnecessary to test the fill_combos since we already know they won't have fill_values.
I would do something like:
def test__ndarray_ndarray(self):
for (lazy0, laz1) in self.lazy_combos:
cubes = iris.cube.CubeList()
cubes.append(self._make_cube(0, dtype=self.dtype, lazy=lazy0)
cubes.append(self._make_cube(1, dtype=self.dtype, lazy=lazy1)
result = cubes.merge_cube()
expected = self._make_data([0, 1], dtype=self.dtype)
self.assertArrayEqual(result.data, expected)
self.assertEqual(result.dtype, self.dtype)
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.
Unless you want to check that after a merge, the resultant data isn't a masked array for some reason
| if data.dtype.kind in 'biu': | ||
| # Integer or Boolean data : No masking is supported. | ||
| msg = 'Non-floating masked data cannot be saved to PP.' | ||
| raise ValueError(msg) |
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.
This was added in #2564 which has now been removed anyway
|
@bjlittle This all looks good. I just want to talk to you about the testing of fill_values when merging ndarrays but otherwise I think this is ready to merge in |
|
Thanks @bjlittle ! |
This PR addresses assorted PP save test failures.
I also ended up bleeding thru into merge and fixing those tests to confirm that merge was behaving correctly for merging masked and ndarray cube payload combos.
We also require to force an
asarray=Falsedefault for_lazy_data.as_lazy_data, otherwisedaskcan cast masked data derived from a proxy as a ndarray.