You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add support for masking/null values to Table class #149
The table docs say: "At this time the astropy.table package has no support for masking and null values. This is to be done." This issue simply records that desire in the issue system.
For the record, when working on ATpy, I found a lot of bugs in the Numpy masked structure array support, some of which were only fixed by Numpy 1.5, so if we want to use the built-in Numpy masked arrays, then this is just an advance warning that we will see failures when using Numpy < 1.5.
@taldcroft - I've started to look at the code to see how we could do this, and I see four main courses of action if we want to implement masking:
Create Masked... versions of all the objects, e.g. MaskedRow, MaskedColumn, MaskedTable which contain the data as a masked Numpy array rather than a Numpy ndarray.
Add a mask attribute to the current table infrastructure and keep the mask and data separate internally (but make it so the user can extract MaskedArrays if needed, like in NDData).
Allow the data to be stored either inside a Numpy ndarray or MaskedArray inside the current elements
Always use MaskedArray instances internally.
One extra complication as I mentioned in a previous comment is that Numpy 1.4 has some bugs in the Masked array implementation.
I started to implement (1), but the issue is that it creates a lot of duplicate code, even if we use inheritance. This would be a bit of a pain to maintain. (4) would be easier to maintain than (3), but given the points about the bugs in the Masked array implementation in 1.4, I would actually lean towards (2) for now (as in NDData). Maybe it would be best to go with that option and then upgrade to allowing (but not requiring) MaskedArrays internally?
@astrofrog and @eteq - One question: what is the support policy for previous versions of numpy? Can we drop 1.4 once 1.7 is out, which maintains support for the current and 2 previous major versions? This might have an impact on the implementation of masking support.
I took a really quick hack at this and was pleasantly surprised that the following patch seemed to work for supporting a masked column:
+import numpy.ma as ma
+
+
+class ColumnMasked(ma.MaskedArray):
"""Define a data column for use in a Table object.
Parameters
@@ -160,9 +163,9 @@ class Column(np.ndarray):
if data is None:
dtype = (np.dtype(dtype).str, shape)
- self_data = np.zeros(length, dtype=dtype)
+ self_data = ma.zeros(length, dtype=dtype)
elif isinstance(data, Column):
- self_data = np.asarray(data.data, dtype=dtype)
+ self_data = ma.asarray(data.data, dtype=dtype)
if description is None:
description = data.description
if units is None:
@@ -172,7 +175,7 @@ class Column(np.ndarray):
if meta is None:
meta = deepcopy(data.meta)
else:
- self_data = np.asarray(data, dtype=dtype)
+ self_data = ma.asarray(data, dtype=dtype)
self = self_data.view(cls)
self._name = name
@@ -187,6 +190,7 @@ class Column(np.ndarray):
return self
I got stuck though trying to do multiple inheritance to put all the masking-independent bits into a ColumnBase class.
-->271classColumn(np.ndarray, ColumnBase):
272 """Define a data column for use in a Table object.
273
TypeError: Error when calling the metaclass bases
metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases
I'm a little wary of option (2) because it feels like we would be rewriting np.ma. You need to provide implementations for every numpy operator and ufunc that depends on masking (e.g. sum(), + - etc). On the other hand I think that pandas may have actually taken that strategy (not 100% sure on this) since the split internal representation (masked or normal array) has its own problems.
Anyway @astrofrog if you want to take a crack at this feel free. I would lean to doing (3) with a masked version of Column and Row (if needed), but it's kind of a hard problem and I can't really see the path to a complete solution (and all possible problems) for any of them. So just pushing ahead and trying something might be the only way.
Looks promising! One slight complication as @mdboom pointed out in #363 is that Numpy masked arrays can't mask individual elements inside a cell in a multi-dimensional column (as far as I understand). If we wanted this, we may need to implement our own approach...
I wonder if metaclasses can work here to dynamically set the inheritance of a Column object to either np.ndarray or np.ma.MaskedArray. That way there would not need to be a distinction. Any thoughts metaclass-master @iguananaut?
Roger the comment from @mdboom on VO and MaskedArray. I would say that we should target missing value support within Table for 0.2 (by the easiest means possible) and leave integration with VO for later. We should definitely take a look at how Pandas tackled this issue since it has good missing value support.
@taldcroft - regarding your question about the Numpy version - I feel like we should try and support 1.4 as much as possible. If it is the case that masking would be very easy with 1.5+, then I say we just make an exception and allow masked tables to only be supported with Numpy 1.5+, but we should still strive to stay compatible with 1.4 for now.
I agree with @astrofrog here - we want astropy to build and work generally with 1.4, but if something in 1.5+ masking really makes it easier, just make sure you add checks at the top of the relevant places for a version, and throw an excpetion that specifically mentions "you need numpy 1.5 or greater" or similar. (Also, be sure to use xfail in any tests.)
Activity
astrofrog commentedon Feb 7, 2012
For the record, when working on ATpy, I found a lot of bugs in the Numpy masked structure array support, some of which were only fixed by Numpy 1.5, so if we want to use the built-in Numpy masked arrays, then this is just an advance warning that we will see failures when using Numpy < 1.5.
eteq commentedon Feb 7, 2012
Ok, good to know - we'd want to make judicious use of
pytest.mark.xfailif that's the internal implementation, then.astrofrog commentedon Aug 25, 2012
@taldcroft - I've started to look at the code to see how we could do this, and I see four main courses of action if we want to implement masking:
Masked...versions of all the objects, e.g.MaskedRow,MaskedColumn,MaskedTablewhich contain the data as a masked Numpy array rather than a Numpy ndarray.maskattribute to the current table infrastructure and keep the mask and data separate internally (but make it so the user can extract MaskedArrays if needed, like inNDData).One extra complication as I mentioned in a previous comment is that Numpy 1.4 has some bugs in the Masked array implementation.
I started to implement (1), but the issue is that it creates a lot of duplicate code, even if we use inheritance. This would be a bit of a pain to maintain. (4) would be easier to maintain than (3), but given the points about the bugs in the Masked array implementation in 1.4, I would actually lean towards (2) for now (as in NDData). Maybe it would be best to go with that option and then upgrade to allowing (but not requiring) MaskedArrays internally?
Do you have any thoughts about this?
taldcroft commentedon Aug 28, 2012
@astrofrog and @eteq - One question: what is the support policy for previous versions of numpy? Can we drop 1.4 once 1.7 is out, which maintains support for the current and 2 previous major versions? This might have an impact on the implementation of masking support.
taldcroft commentedon Aug 28, 2012
I took a really quick hack at this and was pleasantly surprised that the following patch seemed to work for supporting a masked column:
I got stuck though trying to do multiple inheritance to put all the masking-independent bits into a ColumnBase class.
I'm a little wary of option (2) because it feels like we would be rewriting np.ma. You need to provide implementations for every numpy operator and ufunc that depends on masking (e.g. sum(), + - etc). On the other hand I think that pandas may have actually taken that strategy (not 100% sure on this) since the split internal representation (masked or normal array) has its own problems.
Anyway @astrofrog if you want to take a crack at this feel free. I would lean to doing (3) with a masked version of Column and Row (if needed), but it's kind of a hard problem and I can't really see the path to a complete solution (and all possible problems) for any of them. So just pushing ahead and trying something might be the only way.
astrofrog commentedon Aug 28, 2012
Looks promising! One slight complication as @mdboom pointed out in #363 is that Numpy masked arrays can't mask individual elements inside a cell in a multi-dimensional column (as far as I understand). If we wanted this, we may need to implement our own approach...
taldcroft commentedon Aug 28, 2012
I wonder if metaclasses can work here to dynamically set the inheritance of a Column object to either np.ndarray or np.ma.MaskedArray. That way there would not need to be a distinction. Any thoughts metaclass-master @iguananaut?
Roger the comment from @mdboom on VO and MaskedArray. I would say that we should target missing value support within Table for 0.2 (by the easiest means possible) and leave integration with VO for later. We should definitely take a look at how Pandas tackled this issue since it has good missing value support.
astrofrog commentedon Aug 28, 2012
@taldcroft - regarding your question about the Numpy version - I feel like we should try and support 1.4 as much as possible. If it is the case that masking would be very easy with 1.5+, then I say we just make an exception and allow masked tables to only be supported with Numpy 1.5+, but we should still strive to stay compatible with 1.4 for now.
eteq commentedon Aug 28, 2012
I agree with @astrofrog here - we want astropy to build and work generally with 1.4, but if something in 1.5+ masking really makes it easier, just make sure you add checks at the top of the relevant places for a version, and throw an excpetion that specifically mentions "you need numpy 1.5 or greater" or similar. (Also, be sure to use xfail in any tests.)
astrofrog commentedon Nov 28, 2012
This was done in #451, so closing.
embray commentedon Dec 17, 2012
Since this was done in #451, it will be in the 0.2 release and does not belong in the 0.3 milestone.
Merge pull request #149 from astrofrog/allow-frame-instances
Merge pull request astropy#149 from bsipocz/patch-1