Skip to content

Conversation

@lbdreyer
Copy link
Member

@lbdreyer lbdreyer commented Aug 3, 2017

Fix all tests in lib/iris/tests/unit/data_manager/test_DataManager.py so that they all pass.

I have had to remove/change some tests that used the old API (e.g. cube.fill_value).
I have also had to make some fixes to handling of masked constants which hopefully makes sense

@lbdreyer lbdreyer added this to the dask-mask milestone Aug 3, 2017
dm_check = DataManager(self.core_data())
dm_check.data = data
# If the replacement data is valid, then use it but
# without copying it.
Copy link
Member Author

Choose a reason for hiding this comment

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

I am putting this back as this ensures you are doing a deepcopy with a data of the correct shape.
Without this you can do:

>>> dm = DataManager(np.array([0,1,2]))
>>> dm._deepcopy(dict(), data=np.array(0))
DataManager(array(0))

Which shouldn't happen

@lbdreyer lbdreyer force-pushed the dask_dm_deep_copy branch from a2da644 to 6a5a81d Compare August 3, 2017 13:09
"""
if isinstance(data, ma.core.MaskedConstant):
data = ma.masked_array([data], mask=data.mask)
Copy link
Member

Choose a reason for hiding this comment

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

@lbdreyer Doing this promotes the dimensionality of the data e.g.

>>> a = ma.masked_array([1, 2, 3], mask=[0, 1, 0])
>>> v = a[1]
>>> v
masked
>>> r = ma.array([v.data], mask=v.mask)
>>> r
masked_array(data = [--],
             mask = [ True],
       fill_value = 1e+20)

>>> r.shape
(1,)
>>> r.ndim
1
>>> r = ma.array(v.data, mask=v.mask)
>>> r
masked_array(data = --,
             mask = True,
       fill_value = 1e+20)

>>> r.shape
()
>>> r.ndim
0

Copy link
Member Author

Choose a reason for hiding this comment

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

np.asanyarray doesn't work

>>> a = ma.masked_array([8], mask=True)
>>> masked_constant = a[0]
>>> print type(masked_constant)
<class 'numpy.ma.core.MaskedConstant'>
>>> print type(np.asanyarray(masked_constant))
<class 'numpy.ma.core.MaskedConstant'>

Copy link
Member Author

Choose a reason for hiding this comment

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

I also can't do

>>> a = ma.masked_array(8, mask=True)
>>> print a[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/sci/lib/python2.7/site-packages/numpy/ma/core.py", line 2992, in __getitem__
    dout = ndarray.__getitem__(_data, indx)
IndexError: 0-d arrays can't be indexed

Copy link
Member

@bjlittle bjlittle Aug 3, 2017

Choose a reason for hiding this comment

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

@lbdreyer Perhaps my point was lost in the example, but shouldn't line 97 be

data = ma.masked_array(data, mask=data.mask)

instead of

data = ma.masked_array([data], mask=data.mask)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry my last comment makes no sense, that's actually relevant to the tests.

Can we change this line to be
data = ma.masked_array(data, mask=data.mask)
?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, exactly!

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope

>>> orig = ma.masked_array([1,2,3], mask=[True, False, False])
>>> m_con = orig[0]
>>> data = ma.masked_array(m_con, mask=m_con.mask)
>>> print type(data)
<class 'numpy.ma.core.MaskedConstant'>

😿

Copy link
Member

Choose a reason for hiding this comment

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

Apologies, that should be

data = ma.masked_array(data.data, mask=data.mask)

so for your example above it's m_con.data

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! You're right! That does indeed work.

I have now put up a new commit with that change in

@bjlittle bjlittle self-assigned this Aug 3, 2017
@bjlittle bjlittle merged commit a93934c into SciTools:dask_mask_array Aug 3, 2017
@lbdreyer lbdreyer deleted the dask_dm_deep_copy branch July 23, 2018 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants