Bug 123399

Summary: error reading certain fits files
Product: [Applications] kst Reporter: Carrie MacTavish <cmactavi>
Component: generalAssignee: kst
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 1.x   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Carrie MacTavish 2006-03-10 21:49:06 UTC
Version:           1.2.0 (using KDE 3.4.2 Level "b" , SUSE 10.0)
Compiler:          Target: i586-suse-linux
OS:                Linux (i686) release 2.6.13-15-default

for some fits files the data is incorrectly read.  first column of data is read in and then repeated.  example file that i am trying to read can be found here:

http://www.cita.utoronto.ca/~cmactavi/xsht_b03.fits
Comment 1 Andrew Walker 2006-03-13 18:57:40 UTC
Could you provide more details of what you expect to see. Looking at the FITS file you attached the ASCII table seems to consist of 18 columns of data, all of which are identical, with 2001 rows in each column.
Comment 2 George Staikos 2006-03-14 02:51:53 UTC
Can you verify which datasource is being used from the Help->Debug Kst menu?

Thanks
Comment 3 Carrie MacTavish 2006-03-14 02:59:15 UTC
LFILO reader

On Mon, 14 Mar 2006, George Staikos wrote:

[bugs.kde.org quoted mail]
Comment 4 Ted Kisner 2006-03-14 03:08:52 UTC
On Monday 13 March 2006 17:51, George Staikos wrote:
| ------- Additional Comments From staikos kde org  2006-03-14 02:51 -------
| Can you verify which datasource is being used from the Help->Debug Kst
| menu?

I tried this with HEAD (I don't think the datasources have changed much since 
1.2).  The datasource that gets picked up is LFIIO.  In the "new vector" 
dialog, the default start sample is larger than the size of the file.  If I 
set the start to zero and the check the "read to end" box, then it correctly 
reads 2001 elements.

Carrie- did you just select the file and click "ok" (which would use the bogus 
start sample), or does it not read in the vector even if you explicitly 
choose to start at "0" and read to end?

Either way, it seems like the default start sample should be zero (unless the 
user has already opened this file and the state is saved in the global 
config).

-Ted
Comment 5 Carrie MacTavish 2006-03-14 05:26:01 UTC
using data wizard...have 0 to read to end checked...reads in 2001 elements 
fine--this isn't the problem..problem is that kst tells me all 18 cols of 
data (2001 elements each) are identical in this file--with IDL they 
aren't...not sure how the two programs parse fits files differently...

>
> I tried this with HEAD (I don't think the datasources have changed much since
> 1.2).  The datasource that gets picked up is LFIIO.  In the "new vector"
> dialog, the default start sample is larger than the size of the file.  If I
> set the start to zero and the check the "read to end" box, then it correctly
> reads 2001 elements.
>
> Carrie- did you just select the file and click "ok" (which would use the bogus
> start sample), or does it not read in the vector even if you explicitly
> choose to start at "0" and read to end?
>
> Either way, it seems like the default start sample should be zero (unless the
> user has already opened this file and the state is saved in the global
> config).
>
> -Ted
>

Comment 6 Andrew Walker 2006-03-14 18:44:45 UTC
To return to the main issue it would seem that all 18 columns are identical. Carrie, if you don't think is the case could you attach the plot that IDL gives you and the one you get from Kst. It would then simply be a case of checking against the numbers actually in the FITS file.
Comment 7 Carrie MacTavish 2006-03-14 19:03:40 UTC
posted here:

http://www.cita.utoronto.ca/~cmactavi/xsht_b03.fits

is the fits file in question.  with this idl .pro file:

http://www.cita.utoronto.ca/~cmactavi/plot_xsht.pro

i get this output .ps file:

http://www.cita.utoronto.ca/~cmactavi/xsht_b03.ps

reading the same file using kst i get this output:

http://www.cita.utoronto.ca/~cmactavi/kst_xsht_b03.ps


On Tue, 14 Mar 2006, Andrew Walker wrote:

[bugs.kde.org quoted mail]
Comment 8 Andrew Walker 2006-03-14 19:47:17 UTC
The problem is that each column in the ascii table has the same name, which obviously makes it impossible to distinguish the columns by name. Running this file through the FITS verification tool at fits.gsfc.nasa.gov/fits_verify.html results in 52 warnings.
Comment 9 Andrew Walker 2006-03-14 20:28:09 UTC
SVN commit 518642 by arwalker:

BUG:123399 Allow for the posibility that column labels are identical.

 M  +12 -3     lfiio.cpp  


--- trunk/extragear/graphics/kst/src/datasources/lfiio/lfiio.cpp #518641:518642
@@ -117,19 +117,28 @@
           for( i=0; i<iNumCols; i++ )
           {
             iStatus = 0;
-
+      
             sprintf( charTemplate, "%d", i+1 );
             iResult = fits_get_colname( ffits, CASEINSEN, charTemplate, charName, &iColNumber, &iStatus );
             if( iResult == 0 )
             {
+              int iOffset = i;
+              
               strName = charName;
-              _fieldList.append( strName );
+              //
+              // ensure that we don't add duplicates to the _fieldList...
+              //
+              while( _fieldList.findIndex( strName ) != -1 )
+              {
+                strName = QString("%1[%2]").arg( charName ). arg( iOffset );
+                iOffset++;
+              }
             }
             else
             {
               strName.setNum( i );
-              _fieldList.append( strName );
             }
+            _fieldList.append( strName );
 
             iStatus = 0;
             iResult = fits_get_coltype( ffits, i+1, &iTypeCode, &lRepeat, &lWidth, &iStatus );
Comment 10 George Staikos 2006-03-14 21:10:52 UTC
That shouldn't be a problem for backport, and I don't mind doing it as long as 
the user response is positive.  This is a low-impact area that we can easily 
update if there is a regression (in the worst case).

On Tuesday 14 March 2006 14:46, Theodore Kisner wrote:
> As a long term solution, I don't think we should be using fits_get_colname.
>  I think the most general way to read a table is:
>
> 1.  fits_open_table:  open file and move to first HDU containing a table
> 2.  fits_get_hdu_type:  determine whether ascii or binary
> 3.  fits_read_atblhdr:  read number of columns / rows and get all names
>     OR fits_read_btblhdr
> 4.  create fieldnames based on column number and column name (if it exists)
>
> This is what I was going to implement before I was interrupted by this
> commit. Perhaps this is too many changes to be backported to 1.2, but I
> think it is the safest way to read all the brain-dead FITS files out there.
>
> -Ted
>
> On Tuesday 14 March 2006 11:28, Andrew Walker wrote:
> | ------- You are receiving this mail because: -------
> | You are the assignee for the bug, or are watching the assignee.
> |
> | http://bugs.kde.org/show_bug.cgi?id=123399
> | arwalker sumusltd com changed:
> |
> |            What    |Removed                     |Added
> | -------------------------------------------------------------------------
> |-- - Status|UNCONFIRMED                 |RESOLVED
> |          Resolution|                            |FIXED
> |
> |
> |
> | ------- Additional Comments From arwalker sumusltd com  2006-03-14 20:28
> | ------- SVN commit 518642 by arwalker:
> |
> | BUG:123399 Allow for the posibility that column labels are identical.
> |
> |  M  +12 -3     lfiio.cpp
> |
> |
> | --- trunk/extragear/graphics/kst/src/datasources/lfiio/lfiio.cpp
> | #518641:518642 @ -117,19 +117,28  @
> |            for( i=0; i<iNumCols; i++ )
> |            {
> |              iStatus = 0;
> | -
> | +
> |              sprintf( charTemplate, "%d", i+1 );
> |              iResult = fits_get_colname( ffits, CASEINSEN, charTemplate,
> | charName, &iColNumber, &iStatus ); if( iResult == 0 )
> |              {
> | +              int iOffset = i;
> | +
> |                strName = charName;
> | -              _fieldList.append( strName );
> | +              //
> | +              // ensure that we don't add duplicates to the
> | _fieldList... +              //
> | +              while( _fieldList.findIndex( strName ) != -1 )
> | +              {
> | +                strName = QString("%1[%2]").arg( charName ). arg(
> | iOffset ); +                iOffset++;
> | +              }
> |              }
> |              else
> |              {
> |                strName.setNum( i );
> | -              _fieldList.append( strName );
> |              }
> | +            _fieldList.append( strName );
> |
> |              iStatus = 0;
> |              iResult = fits_get_coltype( ffits, i+1, &iTypeCode,
> | &lRepeat, &lWidth, &iStatus );
> | _______________________________________________ Kst mailing list
> | Kst@kde.org
> | https://mail.kde.org/mailman/listinfo/kst
>
> _______________________________________________
> Kst mailing list
> Kst@kde.org
> https://mail.kde.org/mailman/listinfo/kst

Comment 11 Ted Kisner 2006-03-14 21:49:59 UTC
Perhaps we should leave this alone for now.  It fixes the problem that
Carrie was seeing (at least for me).

For 1.3, maybe we should rename LFIIO to "fitstable" to better match
the existing "fitsimage" datasource?  It would also be nice to have
fitstable scan through all HDU's in the  FITS file that contain a
table, and make all these fields available.  Same with fitsimage.

I will play with this in HEAD if no one objects.

-Ted


On 14 Mar 2006 20:10:56 -0000, George Staikos <staikos@kde.org> wrote:
[bugs.kde.org quoted mail]