@mhvk - This is looking promising! I think we can effectively separate the problems of supporting ndarray subclasses like Quantity from supporting arbitrary array-like container classes like Time or other generalized column concepts like FunctionColumn. So perhaps you should rename this PR something like "Support Quantity as Table column", and then stay within that agenda. I'd like to continue #1833 for generalized non-ndarray columns, taking inspiration as needed from #1835.

You might also consider closing #1583 as being superceded by this PR.

@taldcroft - yes, the separation works out quite well; I'm still somewhat amazed that by just using __class__ and __dict__ from an ndarray subclass, one can completely recover Quantity (subclass) objects.

I renamed the PR in the spirit of your suggestion and closed #1583.

p.s. I would have loved to have a more general Column also deal with MaskedArray subclasses, but unfortunately it is not quite as easy as the masked arrays are effectively two arrays, which are not directly tied. In that sense, they still more similar to Time to me, in that a near-trivial conversion between some two-element dtype (with second bool) could be used for MaskedArray<->Column conversion. Anyway, since this subclass does not work, that is for another PR.

Relatedly, currently, Column still uses np.asarray for any data it does not recognize, so masks get stripped, which is fine, but this also means only Quantity and its subclasses can be held for now. It would be nice to generalize this, but that needs some thought about how: the most important requirement would seem to be that slicing does not nothing more than slice the underlying ndarray (i.e., does not also slice metadata like the mask). Anyway, probably best also for another PR!

Right now this doesn't really play with masking, which is perfectly fine. So what about raising an exception for any attempt to have a generalized ndarray column in a masked table?

This will need rebasing now that #1842 has been merged (it makes Travis work again)

@taldcroft - thanks for all the detailed comments. Most I can just implement; I'll think a bit more about how to ensure __getitem__ remains fast.

I'll also see if I can add some checks for MaskedColumn (and will see if I can move a little more common code to BaseColumn)

I've looked at this more today and I think it's going to work. This is pretty impressive stuff. Some of this is somewhat intricate in terms of the logic and use of numpy subclassing techniques, so it wouldn't hurt to add comments to help future readers (both of us included) to understand the flow.

One question we're going to have to answer is how to introduce this API change. Right now if you set a table column with a Quantity you get a Column with the right units. After this PR you will get a QuantityColumn. This is a non-trivial change for anyone that was actually using the former behavior. I'm not sure of the best answer now, just wanted to get this question out there.

OK, thanks! I'll add comments, documentation, and tests... (sure way to find some problems too :-)

Small aside on those comparison functions: while comparing to how we dealt with it in Quantity (through the quantity_helper functions), I noticed just overwriting the comparison functions does not give complete coverage. Currently, one has the same problem that a Column of bool is produced for some other numpy functions. E.g.,

In [5]: np.isfinite(Table([[1., 3., 5.]])['col0'])
Out[5]: 
<Column name='col0' unit=None format=None description=None>
array([ True,  True,  True], dtype=bool)

I'll try to catch this in __array_wrap__ instead; this may even be somewhat faster.

The great thing about this approach is that we can expect success for introducing Quanity columns for most of the other tests, but still, tests are needed... I'm curious if there is any breakage for the high-level table operations like join and grouping.

These are looming in my mind for the non-ndarray columns since they need to go by a different path. I'm pretty sure (at least initially) that the non-ndarray cols won't work as key columns in any high level operations because I'm tied to ndarray methods in the implementation.

On the API change: you're right, I think this requires thought and at the very least a deprecation warning.

Similarly, right now, if you give a plain array and a unit, the result is not a Quantity, which I do not particularly like.

And finally, we have to decide what happens if you set the unit. This is not allowed for a Quantity, but is allowed for a Column. One option would be to allow setting it only if it is None.

I guess this already brings up the larger question touched upon earlier, whether one should have columns in which the unit is only some otherwise meaning piece of metadata (I feel no; that is what meta is for... but I'm greatly helped by being relatively new and unburdened with code which would assume otherwise!).

Agreed that the non-ndarray columns are interesting; ideally, they behave as close as possible to columns. But e.g. on grouping by Time, one may need to tell uses that it only works on, say, the string representation.

@mhvk - what is the status of this? I see it got pushed to 1.0 at some point... Is that definite, or is this actually nearly ready?

Or is there another PR that subsumes this or otherwise makes Table and Quantity work better together?

@eteq - this is being held up mostly by getting things to work with masked columns as well; I've been thinking to try to separate that out better, but have not spent much time recently. I think it is alright to have this for the next major release (with what seems to quite a few other improvements to Table!).

@eteq - I rebased this. Would you like to try it and see if it works in your code? So far, it is rather light on testing (just your examples from #2000)...

@mhvk - this is fantastic! Exactly what I want to see! I think just a few standard tests that they behave like Quantitys should suffice at this point. (and your question on astropy-dev should probably get resolved? Maybe that can be in a later PR, though.)

BTW, is this still a stepping stone to something like #1833 ? I'm particularly interested in trying to stuff SkyCoords into tables...

Also, looks like it needs another rebase, probably due to #2963 ...?

@eteq - thanks! I'll try to add further tests soonish. I'd definitely still want something along the lines of #1833, #1835 or #2790, but I think this is independent of all those.

My real hesitation is that it will not work with MaskedColumn, which is likely to bite the unwary. It may be wise to explicitly raise an exception if one tries.

@mhvk - I've been thinking about tables and revisiting the previous efforts on these lines. I'll be looking at stuff this week.