Skip to content

Conversation

@djkirkham
Copy link
Contributor

@djkirkham djkirkham commented Oct 24, 2017

Replaces #1575

Removes all instances of checking if a MaskedArray has no mask and replacing it with an ndarray.

@pelson
Copy link
Member

pelson commented Oct 24, 2017

Surprised this behaviour isn't tested...

@pelson
Copy link
Member

pelson commented Oct 24, 2017

Relates to #2046.

@pelson pelson added this to the v2.0.0 milestone Oct 24, 2017
@djkirkham djkirkham requested a review from DPeterK October 25, 2017 09:00
Copy link
Member

@DPeterK DPeterK left a comment

Choose a reason for hiding this comment

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

@djkirkham these changes definitely need some testing associated with them...

Thanks for taking this on though!

result = scipy.stats.mstats.mquantiles(data, quantiles, axis=-1,
**kwargs)
if not ma.isMaskedArray(data) and not ma.is_masked(result):
result = np.asarray(result)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reinstated this check since the cube is constructed in this method, and the guard only causes it to be unmasked if the input data was not a masked array, which seems sensible.

fullcube_data[slice_indices_tuple] = data

# Remove the data mask if completely unused.
if not np.ma.is_masked(fullcube_data):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This array is constructed as a masked array at line 223, so if we don't convert it to an ndarray it will output a masked array even if the input is an ndarray.

@djkirkham
Copy link
Contributor Author

djkirkham commented Oct 26, 2017

I've reinstated some of the checks I removed. See my comments on commit f63d950

@pelson
Copy link
Member

pelson commented Oct 27, 2017

LGTM. I'll cross-check the behaviour and ensure there is an appropriate what's new entry.

@pelson pelson merged commit bf89829 into SciTools:master Oct 27, 2017
@pelson
Copy link
Member

pelson commented Oct 27, 2017

Just to confirm that as a result of this PR, the behaviour is unchanged between iris 1.13 and 2.0.

Code to confirm:

from iris.cube import Cube

import numpy as np
import numpy.ma as ma


data = ma.masked_array(np.arange(12).reshape(3, 4), False)
cube = Cube(data)
print(type(cube.data))
print(cube.data)

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.

3 participants