Repository navigation
'FITS_rec.compress' method not properly selection values #3073
Description
Hi,
I cannot get the FITS_rec.compress method to work correctly for selecting rows in a table. Here is an example:
import numpy as N
import astropy.io.fits as AF
# Create a test table
tbl = AF.BinTableHDU.from_columns([
AF.Column(name='test', format='J', array=N.arange(10)),])
sel = tbl.data['test'] < 3 # Selection
print "Selecting test < 3 with boolean array:"
print "selected table:", tbl.data[sel]
print "selected values:", tbl.data[sel]['test']
print "Selecting test < 3 with compress:"
print "selected table:", tbl.data.compress(sel)
print "selected values:", tbl.data.compress(sel)['test']This will print:
Selecting test < 3 with boolean array:
selected table: [(0) (1) (2)]
selected values: [0 1 2]
Selecting test < 3 with compress:
selected table: [(7) (8) (9)]
selected values: [0 1 2 3 4 5 6 7 8 9]
As you can see, the compress method does weird things, at least not expected.
Activity
embray commentedon Nov 6, 2014
I don't think this will likely be supported--most of the usually ndarray methods have never been fully supported here because of how it internally works on converted data arrays. There's no good way (currently) in Numpy for an array subclass to know that
np.compresswas called on the array, and to perform the correct associated manipulations on the underlying column arrays. A better option is to load the table into the AstropyTableclass and not rely on the oldFITS_recinterface for doing "sophisticated" table manipulations. Sorry.embray commentedon Nov 6, 2014
In retrospect I suppose
FITS_reccould override the built-in compress method such that it can work properly on the underlying column arrays as well. But if a (relatively obscure) method like compress were supported, then all built-in ndarray methods should also be supported in this way; I'd welcome a patch for this but overall it's effort better spent on getting astropy Table as the default (and only) returned interface to FITS tables.ycopin commentedon Nov 7, 2014
I acknowledge it might not be worth the effort if Table is supposed to become the standard interface, but in the meantime, maybe it would be preferable to disable unsupported features such as
compress, in the code or at least in the documentation?Edit: This specially true since
compressused to work in previous versions ofpyfits(I used 3.1.2): this bug bit me while upgrading toastropy.io.fitsbut is also present in pyfits 3.3.embray commentedon Nov 7, 2014
I see, thanks for the clarification. I think the fact that it did work is sort of a happy accident in that the tables you were using it with didn't have to use rescaled columns. The newer version treats all columns equally in the implementation whether or not they require rescaling--this vastly simplifies aspects of the implementation but makes scaled and unscaled columns equally susceptible to the bugs inherent in that class's misdesign.
As a temporary workaround you could also do
data.view(np.rec.recarray)to view it as a normal recarray instead of aFITS_rec, so that it won't try to do anything special to handle FITS-isms. It's obviously not ideal though.ycopin commentedon Nov 7, 2014
One point which remains unclear though: while I understand the
FITS_rec.compressis not working properly as it is, and will probably not be fixed, is the selection through a boolean array (tbl.data[sel]) valid? In particular, is it valid to do:embray commentedon Nov 7, 2014
@ycopin I started writing this before I saw your question just now, but this should probably answer it.
For anyone who wants to take a stab at it, this might not actually be too hard to fix (along with properly supporting all the other
ndarraymethods onFITS_rec). I should explain a bit more about howFITS_recis implemented that makes things problematic. At its baseFITS_recis just a structured array that directly reads the structured table data as it is arranged in the FITS file. A FITS table has fixed-width columns so this is fairly straightforward. Unfortunately the raw data in the FITS file is not always the same data that should be presented to the user. Common examples include columns that use the TSCAL and TZERO header keywords to rescale numeric values. Another common example is converting "logical" columns from the characters'T'and'F'to Python True and False values.To this end, the
FITS_recobject has a._convertedattribute, which is just a list of arrays representing columns that have been converted from their on-disk representation to their physical values that are presented to the user. The reason basic selection (such astbl.data[sel]) works correctly is thatFITS_rec.__getitem__has been implemented to understand that there these._convertedcolumns exist, so when slicing a table it attaches the same converted arrays to the sliced table, but also performs the same slice on all the arrays in._converted.However, other Numpy methods that are not aware of this extra structure will just work directly on the underlying raw data as they're designed to, leading to surprising results. This is actually a fairly similar problem faced by Astropy's
Tableclass, though it didn't directly subclassndarrayand so didn't make the mistake of presentingndarraymethods to the user as though they would work normally.I think supporting most
ndarraymethods onFITS_recwould be easy (some easier than others though). For examplecompresscan be supported by also making the appropriatecompresscall on all the arrays in the._convertedlist of the compressedFITS_rec.embray commentedon Jul 6, 2021
I'm not sure when or how (since at least v4.0 if not earlier) but this issue appears to have been fixed (here I tested on main):
Not sure when this was fixed, though 10b2e14 might be a good candidate.