search

radio-astro-tools / spectral-cube

Library for reading and analyzing astrophysical spectral data cubes
http://spectral-cube.rtfd.org
BSD 3-Clause "New" or "Revised" License
95 stars 61 forks source link

Doc: Mask clarification #339

Open pllim opened 7 years ago

pllim commented 7 years ago

In the Masking section, it was stated, "though it is also possible to flip the convention around" but no further clarification is given on how that is possible (keyword option? using ~mask manually?).

Also, since this is an Astropy affiliated package, I'm curious on why the mask definition is the opposite of what is adopted by masking in astropy.table.

keflavich commented 7 years ago

The instructions should specifically mention InvertedMasks, which are described later in http://spectral-cube.readthedocs.io/en/latest/masking.html#inclusion-and-exclusion. I agree there should be a link between these sections.

AFAICT, the convention is arbitrary, and I wasn't aware of the astropy.table convention. The numpy convention is consistent with the astropy one, though. I've always thought of masking as a multiplicative operation, i.e. result = data * mask, which is an inclusion mask.

However, we should avoid arbitrary conventions if possible. I'll regard this issue as a TODO to replace BooleanArrayMask with BooleanIncludeArrayMask and BooleanExcludeArrayMask and similar; these should all be straightforward and low-overhead to implement and easier to understand / less subject to arbitrary convention.

Anyone else, please chime in if you feel differently.

astrofrog commented 7 years ago

I think astropy.table uses the same convention as Numpy masked arrays.

@keflavich - changing the name of the classes might not help with cases like

cube2 = cube1.with_mask(cube1 > 3)

since this still requires the user to know what convention the mask is using?

keflavich commented 7 years ago

@astrofrog that's true. I have a lot that depends on this convention being as it is, but maybe we should add with_include_mask and with_exclude_mask methods too? Though these would not interact in obvious ways with IncludeMask/ExcludeMask objects.

low-sky commented 7 years ago

I also prefer the status quo for masking, namely the mask being an include mask. This plays very well with scipy.ndimage and the morphology operators for mask manipulation and casts well to arithmetic operations. It also operates well with calls to return operations valid values: np.sum(image[mask]) I know it's just a convention, but much of my layering on top of spectral cube also relies on the convention operating this way. I'm not sure why astropy chose such a counterintuitive approach 😉 .

pllim commented 7 years ago

I think with Astropy, it stemmed from Numpy. And it does make sense when your mask is created from DQ arrays, where 0 (False) means a good pixel.