Bug #1831

csiactcopy mistakes FITS columns and rows

Added by Mayer Michael almost 8 years ago. Updated over 7 years ago.

Status:ClosedStart date:08/01/2016
Priority:HighDue date:
Assigned To:-% Done:

100%

Category:-
Target version:-
Duration:

Description

I received a report from Nukri Komin that csiactcopy has a problem when copying a large number of runs using a runlist (and not the full production): some columns in the obs-index and hdu-index files stay empty showing only zeros.

I checked the code and discovered a rather stupid mistake in $CTOOLS/cscripts/csiactcopy.py:line 232:

# Get old size of remote HDU
size = remote_hdu.size()

The size() function returns the number of columns and not the number of rows as intended. Therefore the indexing gets mixed up.
We simply have to replace this line by the following snippet:
size = remote_hdu.nrows()

Recurrence

No recurrence.

Related issues

Related to GammaLib - Action #1857: Remove GFitsTable::size() method Closed 10/03/2016
Related to GammaLib - Action #1858: Remove GFitsImage::size() method Closed 10/03/2016
Related to GammaLib - Action #1859: Rename GFitsHDU::size() method to GFitsHDU::cards() Closed 10/03/2016
Related to GammaLib - Action #1860: Rename GFitsTableCol::length() to GFitsTableCol::nrows() Closed 10/03/2016

History

#1 Updated by Knödlseder Jürgen almost 8 years ago

I looked a bit in the GammaLib code and I think that the size() methods should be removed from the FITS HDU classes to avoid confusion.

The base class method GFitsHDU::size() returns in fact the number of header cards in a HDU.

The GFitsTable::size() method that overloads the GFitsHDU::size() method, returns the number of columns.

And the GFitsImage::size() method that overloads the GFitsHDU::size() method, returns the number of pixels.

The problem with the size() method is that it is not obvious to what size it refers to. And the actual implementations are definitely conflicting.

#2 Updated by Mayer Michael almost 8 years ago

I agree that the size method is a bit confusing. I was expecting something different from it to return. Therefore, having ncols() and nrows() should be sufficient and more clear.
I also stumbled upon a similar problem when accessing the number elements of a GFitsTableCol:
The function returning the number of rows is actually GFitsTableCol.length(), where I intuitively would have expected a size() method. Of course it is a bit more complicated here since an element of a GFitsTableCol can have multiple entries (like we have in response cubes).

#3 Updated by Mayer Michael over 7 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Resolved by #1828. Maybe another issue for the FITS interface is useful.

#4 Updated by Knödlseder Jürgen over 7 years ago

  • Related to Action #1857: Remove GFitsTable::size() method added

#5 Updated by Knödlseder Jürgen over 7 years ago

  • Related to Action #1858: Remove GFitsImage::size() method added

#6 Updated by Knödlseder Jürgen over 7 years ago

  • Related to Action #1859: Rename GFitsHDU::size() method to GFitsHDU::cards() added

#7 Updated by Knödlseder Jürgen over 7 years ago

  • Related to Action #1860: Rename GFitsTableCol::length() to GFitsTableCol::nrows() added

#8 Updated by Knödlseder Jürgen over 7 years ago

  • Status changed from Resolved to Closed

Created issues for renaming of FITS methods.

Also available in: Atom PDF