esheldon / fitsio

A python package for FITS input/output wrapping cfitsio
GNU General Public License v2.0
134 stars 58 forks source link

calling len() adds a HDU to an empty FITS #348

Open ntessore opened 2 years ago

ntessore commented 2 years ago

Calling len() on an empty (i.e. newly created) FITS adds a HDU:

>>> fits = fitsio.FITS('new.fits', 'rw')
>>> len(fits)
1
>>> 0 in fits
True
>>> fits[-1]

  file: test.fits
  extension: 0
  type: IMAGE_HDU
  image info:

The len() call indeed creates the HDU:

>>> fits = fitsio.FITS('another.fits', 'rw')
>>> 0 in fits
False
>>> fits[-1]
ValueError: Requested hdu '-1' not present

This makes the intuitive (well, to me) check len(fits) == 0 for a newly created FITS unusable.

esheldon commented 2 years ago

There is always at least one extension in a FITS file, so I think this is the right behavior. Maybe we could add a method that does something like len(fits) == 1 and not fits[0].has_data() to indicate no data other than header entries are present

ntessore commented 2 years ago

There is always at least one extension in a FITS file

A FITS file, yes, but the second code example shows that this is not true for a FITS object. Creating a FITS object and not doing anything to it does initially result in something that has no HDUs.

I suppose it goes back to the question of whether or not a FITS object corresponds a valid FITS file on disk (#323).

But in any case, I would argue that calling len() should never modify an object, or start writing things to disk.

esheldon commented 2 years ago

I think the bug here is that fits[-1] doesn't work.

The reason len creates the hdu list is because FITS files can have thousands of extensions, so this is done lazily. It may be there is a better way, but do note that it is not possible to determine the number of extensions without running through the whole file, which can be expensive.