Fits merge - #495

138k additions? seriously? ;-)

This will make the linux package managers happy! I will try and review this broadly, and wil test out the --use-system-cfitsio option.

@astrofrog Most of that is just CFITSIO and its cross product of source modules for getting/putting each data type supported by FITS. I tried to hack it down to a bare minimum required for the pyfits compression module, but that turned out to be too unmanageable from a maintenance perspective.

Looks like there are some test failures related to the new compression module not building/working properly in Python 3. I'll look into this.

.I got the following error when trying to compile with --use-system-cfitsio:

/usr/bin/gcc-4.2 -fno-strict-aliasing -fno-common -dynamic -pipe -O2 -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -I/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/numpy/core/include -I/opt/local/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -c astropy/io/fits/src/compressionmodule.c -o build/temp.macosx-10.6-x86_64-2.7/astropy/io/fits/src/compressionmodule.o
astropy/io/fits/src/compressionmodule.c: In function ‘configure_compression’:
astropy/io/fits/src/compressionmodule.c:513: error: ‘FITSfile’ has no member named ‘cn_gzip_data’
astropy/io/fits/src/compressionmodule.c:523: error: ‘FITSfile’ has no member named ‘cn_gzip_data’
error: command '/usr/bin/gcc-4.2' failed with exit status 1

I presume this is an issue to do with the CFITSIO version? This makes me think, should we check the versions of system libraries and raise a proper error if they are too old? (the same point applies to expat and wcslib). Or is it basically up to the user to make sure the versions work out? I guess we should explicitly list the required versions in the documentation.

There are some version checks, but it looks like I missed that. I can certainly fix it.
Specifically, support for the GZIP_COMPRESSED_DATA column was added in CFITSIO 3.28. On the PyFITS end I already check for that version before doing anything with the GZIP_COMPRESSED_DATA column. But in the C source it doesn't do the same check before trying to assign to that struct member.

Got it building in Python 3, but now I'm getting a segfault. The segfault itself doesn't occur in my compression code, but rather from Numpy. That doesn't necessarily mean there's not a bug in my code somewhere. It's just hard to see it as of yet.

No, I think it is Numpy. I'm using Numpy 1.5.1 on my Python 3.2 installation, apparently. And this is a bug I've seen before where Numpy assumes that an array descriptor's shape will always be a tuple, and is inconsistent about making sure to convert scalar values (such as an int) to a tuple when constructing arrays and such. Should be easy enough to fix.

Oh, I still need to fix @astrofrog's issue with compiling with a <3.28 version of cfitsio.

@astrofrog With the last set of commits I should have improved support for older versions of CFITSIO. Specifically, 3.09 and up should all work. YMMV with earlier versions. Could you give the --use-system-cfitsio build another go? And if it still doesn't work you'll have to find out for me what version of cfitsio you're using.

Also, where do you think would be the best place to document CFITSIO version support?

@iguananaut - I think we should document it here: http://docs.astropy.org/en/latest/install.html#external-c-libraries

Works for me! This looks good to go once you've added a note in the docs about the version number.

Any idea what these Travis failures are all about? It says it can't run setup.py egg_info?

I don't understand, it's been doing that with some other PRs too. I will look into it.

Well, when I run the tests locally with Python 3.2 all the fits tests pass, but I get several errors in the logging tests due to, for some reason, 'asctime' being missing from LogRecords.

Here's how to reproduce the error - it's actually to do with numpy, not astropy:

$ pip-3.2 install --upgrade "numpy<=999.99" --use-mirrors
[snip]
Running from numpy source directory.Traceback (most recent call last):

  File "<string>", line 14, in <module>

  File "/Users/tom/tmp/build/numpy/setup.py", line 214, in <module>

    setup_package()

  File "/Users/tom/tmp/build/numpy/setup.py", line 207, in setup_package

    configuration=configuration )

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/core.py", line 186, in setup

    return old_setup(**new_attr)

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/core.py", line 148, in setup

    dist.run_commands()

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/dist.py", line 929, in run_commands

    self.run_command(cmd)

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/dist.py", line 948, in run_command

    cmd_obj.run()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/egg_info.py", line 8, in run

    self.run_command("build_src")

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/cmd.py", line 313, in run_command

    self.distribution.run_command(command)

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/dist.py", line 948, in run_command

    cmd_obj.run()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/build_src.py", line 152, in run

    self.build_sources()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/build_src.py", line 163, in build_sources

    self.build_library_sources(*libname_info)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/build_src.py", line 298, in build_library_sources

    sources = self.generate_sources(sources, (lib_name, build_info))

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/build_src.py", line 385, in generate_sources

    source = func(extension, build_dir)

  File "numpy/core/setup.py", line 694, in get_mathlib_info

    st = config_cmd.try_link('int main(void) { return 0;}')

  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/distutils/command/config.py", line 243, in try_link

    self._check_compiler()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/command/config.py", line 75, in _check_compiler

    c_compiler=self.compiler)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 832, in new_fcompiler

    c_compiler=c_compiler)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 814, in get_default_fcompiler

    c_compiler=c_compiler)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 763, in _find_existing_fcompiler

    c.customize(dist)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 502, in customize

    get_flags('arch', aflags)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 491, in get_flags

    flags.extend(getattr(self.flag_vars, tag))

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/environment.py", line 37, in __getattr__

    return self._get_var(name, conf_desc)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/environment.py", line 51, in _get_var

    var = self._hook_handler(name, hook)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 698, in _environment_hook

    return hook()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/intel.py", line 63, in get_flags_arch

    v = self.get_version()

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/fcompiler/__init__.py", line 430, in get_version

    version = CCompiler.get_version(self, force=force, ok_status=ok_status)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/ccompiler.py", line 26, in <lambda>

    m = lambda self, *args, **kw: func(self, *args, **kw)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/ccompiler.py", line 458, in CCompiler_get_version

    status, output = exec_command(version_cmd,use_tee=0)

  File "/Users/tom/tmp/build/numpy/build/py3k/numpy/distutils/exec_command.py", line 197, in exec_command

    if _with_python and (0 or sys.__stdout__.fileno()==-1):

ValueError: underlying buffer has been detached

I wonder what changed suddenly. Either numpy 1.6.2 was only recently added to pip, or maybe they did a hot swap? In any case, this is not something I think we can do anything about, so I would trust the staging branch in Jenkins over Travis.

Yup, I'm baffled by that. I'll go ahead and try my staging branch.

@iguananaut - did you try and start the MacOS X staging build from the jenkins interface? (https://jenkins.shiningpanda.com/astropy/job/astropy-staging-iguananaut-osx-10.7-multiconfig/NV=1.5.1,PV=3.1/12/console - it says 'started by user iguananaut').

Yeah, I did. Did I screw it up? Sorry if so.

Edit: I went ahead and deleted the project. I didn't realize the OSX builds weren't hosted by Shining Panda--I'm now guilty of my own pet peeve with Jenkins--sorry about that.

No worries! I forced a new build and it all passes: https://jenkins.shiningpanda.com/astropy/job/astropy-staging-iguananaut-osx-10.7-multiconfig/

I tried upgrading to this branch, and I am no longer able to read some files I have that are in Rice compressed format, for example http://www.mpia.de/~robitaille/astropy/w20050513_01570_sf_st.fit

In [1]: from astropy.io import fits

In [2]: hdulist = fits.open('w20050513_01570_sf_st.fit')

In [5]: hdulist[1].data
ERROR: RuntimeError: Error reading elements 1 thru 3727 from column 1 (ffgclb). [astropy.io.fits.hdu.compressed]
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
/Volumes/Scratch/UKIDSS/frames/stack/<ipython-input-5-507714abff29> in <module>()
----> 1 hdulist[1].data

/Volumes/Raptor/Library/Python/2.7/lib/python/site-packages/astropy/utils/misc.pyc in __get__(self, obj, owner)
    256         key = self._fget.func_name
    257         if key not in obj.__dict__:
--> 258             val = self._fget(obj)
    259             obj.__dict__[key] = val
    260             return val

/Volumes/Raptor/Library/Python/2.7/lib/python/site-packages/astropy/io/fits/hdu/compressed.pyc in data(self)
    901     def data(self):
    902         # The data attribute is the image data (not the table data).

--> 903         data = compression.decompress_hdu(self)
    904 
    905         # Scale the data if necessary


RuntimeError: Error reading elements 1 thru 3727 from column 1 (ffgclb).

What has changed that is causing this to fail?

What has changed that is causing this to fail?

Well the almost complete rewrite of the compression module would be a start :)

What version of CFITSIO did you build/link with? You can double check this with
>>> fits.compression.CFITSIO_VERSION

For what it's worth the data in that file is loading for me using the built-in cfitsio and looks fine (assuming that it's supposed to have those dots in the middle of each star--I obviously have no idea what that's supposed to mean).

The holes in the stars are normal (just saturation). I did a clean install and it still doesn't work here. I get:

In [6]: fits.compression.CFITSIO_VERSION
Out[6]: 3.3

This is on MacOS 10.7. This is strange...

Strange indeed--I realized that when I tested this yesterday I was testing with pyfits, not astropy. But even with this PR on astropy it's working for me. d3fc78c fixed a bug that looked just like that, but you should have that in your copy by now.

I guess there's nothing to do but start knocking out other possibilities. OSX is a start--I'll try testing on there. Also, what Numpy version are you using? I have:

Astropy version 0.2.dev2294
with Numpy version 1.6.1
Decompressed with CFITSIO version 3.30

Here's mine:

In [4]: astropy.__version__
Out[4]: '0.2.dev2294'

In [6]: np.__version__
Out[6]: '1.8.0.dev-a289be8'

In [7]: fits.compression.CFITSIO_VERSION
Out[7]: 3.3

I can try and downgrade numpy

Ah, it works with Numpy 1.6.1 and 1.6.2, but not the latest developer version. Does this help identify the issue?

I'm going to try and git bisect to find out when the breakage occurred

Okay, I got the same error using the latest dev Numpy. So that at least partially explains that. I should try to fix it though.

Don't bother with the bisect--I'm pretty sure it has to do with this: numpy/numpy#438
I knew this change was being made, but now it's more annoying than I realized. I think this is going to cause some problems in pyfits' implementation where Numpy 1.8 is concerned. Specifically, it's dropping any references to the raw_data base array that holds the entire table data including the heap which is kind of bizarre.

I did bisect anyway for fun (never had a chance before), and this is what I get:

e411631e136bcdf53ef421102d8cb989beaacf08 is the first bad commit
commit e411631e136bcdf53ef421102d8cb989beaacf08
Author: 87 <[email protected]>
Date:   Tue Aug 30 21:44:54 2011 +0200

    BUG: PyArray_SetBaseObject didn't set proper base when collapsing views

:040000 040000 5edbf90d3f1b4b54215490cc823b7381f3782af5 69023dcfcc56be0a84454205e3c8b93336ae1ad1 M  numpy

Interestingly, the commit I found was much earlier than the issue you pointed to.

I guess it will also cause issues with Numpy 1.7.0 once it's out?

Just confirming that I get the error with Numpy 1.7.0b2.

numpy/numpy#2746 fixes the issue. So for now I'll assume it will get pulled.

Awesome, thanks for following this up!

I think this is going to cause some problems in pyfits' implementation where Numpy 1.8 is concerned. Specifically, it's dropping any references to the raw_data base array that holds the entire table data including the heap which is kind of bizarre.

^ Can you guys elaborate on this? I'm glad if this has been fixed for 1.7/current master, but the .base handling code may change again in the future and this statement is very worrisome. Even with the original pre-fixup code that was originally in 1.7, it shouldn't have been losing references to arrays that are actually holding data...

@iguananaut - just to confirm, updating to the latest numpy dev version now makes this PR work properly.

@njsmith Thanks for checking in on this. Let me try to clarify. FITS tables have a strange structure for supporting variable length fields by which it includes a "heap" section to the data after the data itself. Variable length columns then contain just offsets into the heap portion, for the value of that column for each row. So basically the data consists of a table, followed by some additional arbitrary-length data.

What I'm doing, in order to keep all the data together, is to create a byte array of all the data, then taking a slice of that for the portion that contains the actual table and viewing it as a recarray with the appropriate columns. The details of retrieving the variable length data from the heap are handled elsewhere.

The problem here is that I have some C code that, when passed one of these recarrays, needed to be able to traverse the bases to find the original byte array containing both the table and heap data. It was references to this array that were being lost.

To complicate matters, that data is normally mmap'd. What was not lost was the reference to the original mmap.mmap object that was used as the original array's buffer. But the C code was expecting to find an ndarray of the appropriate size in the bases, and doesn't know what to do when it instead encounters an mmap object.

Going ahead and merging this. Travis is happy with it now, as is my Windows staging branch and the OSX staging branch. Linux should be fine.