POC: minimal masking for Quantity - #6031

Other reasons to use NaN instead of a new MaskedQuantity class that has a separate mask array:

• Works with binary format serialization (FITS, HDF5) right out of the box. Zero changes required.
• Familiar to pandas users. Pandas does not have the explicit concept of a mask, but does have an is_null() method as a proxy to that. Pandas does not support an explicit mask in the initializer. One can only directly mark data as missing via setitem. I agree with all this (one and only one way to do it).
• Only one class Quantity. The whole Column/MaskedColumn dichotomy is a pain.

This brings up an important philosophical distinct that the data are really missing as opposed to being hidden by a mask. Once an item is marked as missing there is no getting it back. I think this is actually a good thing - it removes this concept of "what is behind the mask", which I have seen to be a source of confusion. Also removes the concept of "hardening" the mask.

Rebased this off of #6028 to include the mask_val protocol established there. Note that with just a few lines of new code there is full support for QTable now!

@mhvk - any chance you can decide in the next few days whether you would be OK with the concept of using np.nan as a masking sentinel in Quantity? I'm keen to try to get this ready for 2.0, but if there isn't enough time for you to decide we can push this off until after 2.0.

@taldcroft - I'm sorry I have been so long -- I'm trying to ensure that the __array_ufunc__ and proper motion related PRs get in, and it doesn't help being at a workshop for the coming few weeks!

I'm still intrigued by this, but worry that things are not quite as simple: with the current PR, any method like .sum() will fail if any elements are masked. In principle, one might replace the methods with nan* equivalents, but those are substantially slower (since the whole array has to be scanned), so one wouldn't want to do that for the normal case.

@taldcroft - I've been working a bit with tables with lots of masked items the last few days, and that made me realize ever more how great it would be if quantities and coordinates could indeed be masked. I wonder if it would be an idea to proceed more incrementally: for anything with a mask attribute, we use that, but if a given class doesn't have that, we provide a default .info.mask attribute that simply is a boolean array. This default would get used in printing a table and set by table join, etc., but doesn't have any other effect. For Quantity, though, it uses your scheme of being isnan and setting it writes a nan. The advantage for Quantity is that this only affects the QuantityInfo class, not Quantity itself, which means that we're still free to pursue a MaskedQuantity that has a non-NaN based mask more like MaskedArray.

@mhvk -

About providing a generic .info.mask attribute as a default, I'm worried that having something that looks like a mask but doesn't really work as a mask in most circumstances is asking for trouble. Users now understand that a mixin column SkyCoord object is a bona-fide SkyCoord. So they would expect to be able to take that column out of the table and have it be masked. Having said that, I do appreciate the merit in incremental approaches, getting something done, and being practical in the face of difficult problems.

Note that with Coordinates there is really no reason to not use nan in the sense that there is no int representation values. So the question is how difficult it is to get all the rest of the machinery to not choke / warn when the coordinate representation object has nan. Certainly not easy, but maybe possible?

BTW this reminds me of more reasons that the Quantity nan approach is nice.
Having a distinct Quantity and MaskedQuantity will introduce new pain just as the whole Column / MaskedColumn dichotomy is annoying. We have this machinery in table to decide whether to create a Column or MaskedColumn, and all this annoyance of upgrading a table from unmasked to masked (where all the columns need to be upgraded etc).

Basically with the nan approach there is just one class and you don't need to decide up front about the class nor transform between the two. It's just software, and everything is possible, but one class is simpler than two.

Hi humans 👋 - this pull request hasn't had any new commits for approximately 5 months. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please close this and open an issue instead to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If you believe I commented on this issue incorrectly, please report this here.

⏰ Time's up! ⏰

I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.