Ndmask - #224

In any function that takes an NDData object (say a spectrum spec), would one need to do something like spec.mask.mask to access the mask itself?

@kbarbary what do you mean by access? Can you give an example?

@wkerzendorf Imagine a photometry function that takes Image objects and needs to access the mask attribute to see whether pixel (i, j) is masked. One would have to do image.mask.mask[j, i], correct?

What I'm really saying is: don't we want a getitem() method so that one can just do image.mask[j, i]?

Initially that was done because it inherited from ndarray. But sure, do you mind if we put this onto the 'to be done`-pile and see what fundamental problems there exist first?

@wkerzendorf A second question: again, imagine a photometry function that takes Image objects. Would the function have to test to determine what kind of mask the Image object has (e.g., BoolMask, etc...) in order to know how to treat it? Are we promising for every mask type that where the mask evaluates to False the datum is good? [Perhaps if you implemented another kind of mask this would become clearer]

Would it be simpler to promise that the mask attribute of NDData is always a bool array and have a separate attribute, say flags, for more complex flags? I'm just not sure what the use case is for including other flags in mask. Again, perhaps it would be clearer with another kind of mask implemented.

@kbarbary raises a good point that I hadn't considered before - whether we want to separate flags from masks. This would make the mask implementation simpler, because it's easy to know how to combine bool masks, and flags just don't get combined at all. One reason for doing this is that one might want to have different flags for good data - the way we are going right now, we can only have different kinds of bad data, but one kind of good data. I've seen many instances in astronomy of flags on good data. What do other people think?

@astrofrog @kbarbary I asked that point slightly differently earlier do we want to call it flags or masks. But actually splitting it is a good idea.

What would we do in the use case that we have different flags of 2=saturated and 4=too close to the edge and want to mask all the pixels that are too close to the edge. Should flags and mask talk?

In addition, just talking about a boolean mask. How does addition work. When we add two pixels and one of them is bad what should the output be? mask=not masked and pixel just from the good frame; mask=mask and pixel=na?

What are the thoughts on this?

If we go down the road of having bool-only masks, and flags as separate information, we should look into what Numpy does for masked arrays with respect to operations:

In [1]: import numpy.ma as ma

In [2]: a = ma.array([1,2,3,4,5], mask=[1,0,0,1,0])

In [4]: b = ma.array([6,7,8,9,10], mask=[1,0,0,0,1])

In [5]: a
Out[5]: 
masked_array(data = [-- 2 3 -- 5],
             mask = [ True False False  True False],
       fill_value = 999999)

In [6]: b
Out[6]: 
masked_array(data = [-- 7 8 9 --],
             mask = [ True False False False  True],
       fill_value = 999999)


In [7]: a + b
Out[7]: 
masked_array(data = [-- 9 11 -- --],
             mask = [ True False False  True  True],
       fill_value = 999999)

It looks like the default is to basically do a NAND operation on the masks. It might be worth being consistent with them?

In your scenario, I might want to do:

d = NDData(...)
...
d.mask = d.mask & (d.flags == 4)

i.e. just explicitly set the masks, so that they don't have to talk to each other internally. What do you think?

I agree with @astrofrog about explicitly setting masks, e.g.,

d.mask = d.mask & (d.flags == 4)

This makes explicit exactly what is being masked. In a different operation, one might want to mask saturated stars (d.flags == 2) rather than things too close to the edge. Letting various operations create their own masks based on the mask and flags attributes seems the most straightforward and flexible route.

Also agree about the default behavior of mask arithmetic (to coincide with numpy.ma behavior). However, keep in mind that some operations will want to do other things. I'm specifically thinking of image stacking: If a pixel is masked in one exposure but valid in a second exposure, then in the stacked image, the pixel should be valid (but assigned an appropriate larger uncertainty than pixels with two valid exposures contributing). So in this case the output mask would be the result of an AND operation. (@astrofrog I think you meant the numpy.ma default behavior is an OR operation?)

okay we'll have masks and flags. What problems could we run into with this? Is this to many attributes for nddata?

Regarding the MA behaviour I think that is not trivial. Numpy is currently struggling with that and I think this might be worth a read: https://github.com/numpy/numpy.scipy.org/blob/master/NA-overview.rst

Interesting discussion on NumPy masked arrays, but I don't think we have to worry too much about it. With NDData we are talking about a more limited set of possible data types to support than numpy in general. We can make it clear in documentation that when you have two NDData objects and add them nddatasum = nddata1 + nddata2, the resulting mask will be nddatasum.mask = nddata1.mask | nddata2.mask. However, for a function like stack_images() that might take a bunch of Image objects, the resulting mask will follow different rules (e.g., an AND operation on input masks).

In summary I think it's fine to choose a convention for basic arithmetic operations on NDData (and I think OR is a reasonable one) but keep in mind that other functions will do other things, and should simply document how they treat masks.

Personally, I don't think 'too many attributes' is a problem. A lot of NDData objects will probably have flags = None, but its there if it needs to be used.

If we decide to separate mask and flags can we close this PR? (since mask will always be a bool array, we don't need a Mask class, right?)

closing this pull request as implemented in #303