IPP Software Navigation Tools IPP Links Communication Pan-STARRS Links

Opened 22 years ago

Closed 21 years ago

Last modified 21 years ago

#226 closed defect (fixed)

FITS API suggested changes.

Reported by: robert.desonia@… Owned by: eugene
Priority: high Milestone:
Component: PSLib SDRS Version: unspecified
Severity: normal Keywords:
Cc:

Description

Per our discussion, here is the API I proposed on the 22nd of Oct.

psFits* psFitsAlloc(const char* filename);
opens a fits file and positions it to the primary HDU.

bool psFitsMoveExtName(psFits* fits, const char* extname);
moves the HDU to the specified extension name. If extname does not exist,
shoul we just fail?

bool psFitsMoveExtNum(psFits* fits, int extnum, bool relative);
moves the HDU to the specified extension number.

int psFitsGetExtNum(psFits* fits);
returns the current HDU number (i.e., file position)

int psFitsGetSize(psFits* fits);
returns the number of HDUs in the file.

int psFitsCreateExt(psFits* fits, psFitsType type, const char* name);
creates a new HDU on the end of the file with the given extname and type.
the psFits object is positioned to the new HDU.

psFitsType psFitsGetExtType(psFits* fits);
gets the current HDU's type (table or image).

psMetadata* psFitsReadHeader(psMetadata* out, const psFits* fits);
reads the HDU header (returns psMetadata instead of psHash to make symetric
with psFitsWriteHeader). If out=NULL, a new psMetadata is created.

bool psFitsWriteHeader(psMetadata* out, psFits* fits);
writes the values of the metadata to the current HDU header

psImage* psfitsReadImageSection(psImage* out, psFits* fits, psRegion region, int z);
reads an image, given the desired region and z-plane.

bool psFitsWriteImageSection(psFits* fits, const psImage* input, psRegion
region, int z);
writes an image, given the desired region and z-plane

psMetadata* psFitsReadTableRow(psFits* fits, int row);
reads a table row, as specified.

psArray* psFitsReadTableColumn(psFits* fits, const char* colname);
reads a table column.

psVector* psFitsReadTableColumnNum(psFits* fits, const char* colname);
reads a table column of numbers

psArray* psFitsReadTable(psFits* fits);
returns a psArray of psMetadata objects representing the whole table

bool psFitsWriteTable(psFits* fits, psArray* table);
accepts a psArray of psMetadata and writes it to the current HDU. If the
current HDU is not a table type, this will fail and return FALSE.

Change History (19)

comment:1 by eugene, 22 years ago

Resolution: fixed
Status: newclosed

I've modified the current draft of version 09 of the SDRS to match the listing
below, with a few minor mods:

  • added back the psFitsReadHeaderSet (reads collection of headers into a psHash

of psMetadata collections)

  • added psFitsReadTableRowRaw and psFitsReadTableRaw which simply return void*

data blocks and the number of bytes for each rather than filling out a
psMetadata structure. These functions will be used as the first part of
psFitsReadTableRow, and also will be used in the case where we have a specific
structure defined to match the table columns. In this case, it is more
appropriate to have the data as a continuous stream of bytes which can be
pointed at with the corresponding structure.

comment:2 by robert.desonia@…, 22 years ago

Resolution: fixed
Status: closedreopened

I found in implementation that psFitsCreateExt really probably should be split
into psFitsCreateImage and psFitsCreateTable, as to create a proper image or
table HDU, I need unique information.

The problem is that I can't write the keywords out of order. I originally
planned to just create an HDU with just the SIMPLE/XTENSION and EXTNAME
keywords, and during the write command, fill in the other required fields (e.g.,
BITPIX). CFITSIO, and maybe the FITS standard itself, dictates that the
keywords need to be written in a specific order though, i.e., EXTNAME can't be
written before BITPIX is and I don't know BITPIX at that time.

psFitsCreateImage(psFits* fits, const char* name, psU32 numCols, psU32 numRows,
psU32 numPlanes, psType pixelType);

psFitsCreateTable(psFits* fits, psFitsType tableType, psMetadata* firstRow);

The other solution is to make setting of the extension name a separate call. In
that case, One would have:

bool psFitsCreateExt(psFits* fits);
bool psFitsSetExtName(psFits* fits, const char* name);

and the sequence required would be:

psFitsCreateExt(fits);
psFitsWriteImage(fits, input, {0,0,0,0}, 0);
psFitsSetExtName(fits, "OTA1");

What do you think?

comment:3 by Paul Price, 22 years ago

Owner: changed from Paul Price to eugene
Status: reopenednew

comment:4 by eugene, 22 years ago

Status: newassigned

I have the following counter-proposal: the problem is entirely with the writing
stage of the problem. Normally, you are either writing a complete new data
block (header + data) or you are updating the data or header segments. In the
first case, you already have both the header and the data available. In the
second case, you are restricted to the circumstance where the file size will not
change (since you are over-writing the existing file structure). Perhaps we
should simply drop the 'CreateExt'-style functions and instead define the following:

bool psFitsWriteImage(psFits* fits, psMetadata* header, psImage* input);
bool psFitsWriteTable(psFits* fits, psMetadata* header, psArray* table);

bool psFitsUpdateImage(psFits* fits, psMetadata* header, psImage* input,

psRegion* region, int z);

bool psFitsUpdateTable(psFits* fits, psMetadata* header, psMetadata* table,

int row);

bool psFitsUpdateHeader(psFits* fits, psMetadata* header);

The first two functions would construct both the header and data segments, and
would require the pointer to be at the end of the file, or will truncate the
file to this extension

The last three functions would overwrite the existing data, but would require
that the file size not change.

what do you think?

comment:5 by robert.desonia@…, 22 years ago

At first glance, this seems to solve the problem. I'll implement it this way
and let you know if I find any "stumbling blocks" with it.

-rdd

comment:6 by robert.desonia@…, 22 years ago

Small follow-up:

Should we specify the number of z-planes to create in a HDU with
psFitsWriteImage? If not, how does one create a data cube of images?

-rdd

comment:7 by eugene, 22 years ago

good point. It needs to look like this, then:

bool psFitsWriteImage(psFits* fits, psMetadata* header, psImage* input, int depth);

comment:8 by eugene, 22 years ago

I've cleaned the wording in the SDRS to reflect the addition of the 'depth'
argument.

comment:9 by eugene, 22 years ago

I've been going over the FITS I/O APIs and I have a couple of questions for
Robert DeSonia. I want to have the SDRS reflect what you are coding, and make
sure the usage is clear. Also, in the last released SDRS, there were a few TBD
questions, which I'll re-raise here:

  • psFitsWriteHeader / psFitsUpdateHeader

we have mentioned both names, and it is not clear which is being used, or if
both are available. It is possible that the only reason to write to the header
segment only (ie, not with the data segment) is to update values in the header
block -- all headers are created with one of the psFitsWriteTYPE functions,
which create and write both header and data block at the same time. On the
other hand, this prevents people from using the psLib tools to write a new type
of FITS entry (ie, not IMAGE, TABLE, BINTABLE). The new type would have to have
a corresponding psFitsWriteTYPE function defined. On the other other hand, we
could get around this by defining a function psFitsWriteRaw (psFits *fits,
psMetadata *header, char *data) which would write a data block plus a header,
under the recognition that it would be the user's responsibility to
appropriately format the data segement (though padding could be taken care of
based on Naxis, Naxis(i), and bitpix). thoughts? are you currently coding both
psFitsWriteHeader and psFitsUpdate header or only one?

  • psFitsMoveExtName / psFitsMoveExtNum : do these need to return a pointer or

can they simply return a boolean status? (ie, do we risk CFITSIO reallocing the
pointers?)

  • what numbering sequence does CFITSIO use: PHU == 0, 1st extension == 1?
  • psFitsWriteImageSection vs psFitsUpdateImage : which name are you coding? do

you have a preference?

comment:10 by eugene, 22 years ago

Owner: changed from eugene to robert.desonia@…
Status: assignednew

comment:11 by robert.desonia@…, 22 years ago

Status: newassigned

First, let's just use psFitsWriteHeader. This function can determine if the key
is new or an update. The only problem I see is it wouldn't allow duplicate keys
to be made (i.e., multiple keys with the same name), but I don't know if that is
really something one would desire (too easy to abuse).

psFitsMoveExtName / psFitsMoveExtNum: these return a bool in the implementation.

the psLib API assumes the primary PHU to be 0, while CFITSIO defines it as 1.
If you would rather have the primary HDU to be 1, let me know and I can remove
the addition or subtraction of 1 in my code.

psFitsWriteImageSection vs psFitsUpdateImage : I am using the names in comment 3
and 6 below, i.e., psFitsWriteImage to create a new image HDU and
psFitsUpdateImage to update the current HDU's image. Also would recommend
changing psFitsReadImageSection to just psFitReadImage ('Section' is not really
needed and just makes the name longer).

comment:12 by robert.desonia@…, 22 years ago

Owner: changed from robert.desonia@… to Paul Price
Status: assignednew

comment:13 by robert.desonia@…, 22 years ago

Owner: changed from Paul Price to eugene

comment:14 by robert.desonia@…, 22 years ago

Another detail: should psFitsWriteTable always create a BINARY table (or ASCII
table), or should that be an additional parameter to the function?

I assume that you desire support for both types of FITS tables in the read
functions, correct?

-rdd

comment:15 by robert.desonia@…, 22 years ago

For the function psFitsReadTableColumn, which returns a psArray, do you want me
to return every value as a string? If not, there is no means for the user to
know what the void* in the psArray is pointing to.

BTW: CFITSIO has a function, fits_read_col_str, that returns everything as a
string, so this should be easy to do.

-rdd

comment:16 by robert.desonia@…, 22 years ago

For psFitsWriteImage / psFitsWriteTable, should there not be another parameter
giving the extension name of the HDU being created?

comment:17 by eugene, 22 years ago

a few responses to the flurry of comments:

  • psFitsWriteHeader should not be allowed to create multiple data segments with

the same EXTNAME value, so that is not a problem.

  • I've changed the docs to reflect the bool return values for psFitsMoveExtName/Num
  • Let's keep PHU = 0
  • the docs now use 'psFitsWriteImage', 'psFitsUpdateImage', and 'psFitsReadImage'
  • I agree with your proposal: psFitsReadTableColumn should convert all values to

string (since with have the psFitsReadTableColumnNum).

  • psFitsWriteTable / psFitsWriteImage: you bring up a point which is related to

a few issues that have been bouncing in my head a bit already:

  • We are defining these functions to take psMetadata constructs representing

the image header. But, the psMetadata structure can include entries which are
invalid FITS header keyword + value entries. examples: keys with names > 8
chars. data values more complex than the primatives.

  • functions like psFitsWriteImage and psFitsWriteTable include information in

the FITS header which define the data layout, information which must be
consistent with the data in the structured data type. (ie, the header must
include all of the column identifying tags for a table, or NAXIS1, etc for an
image, but the image structure also includes this information).

The question in my mind is: how do we enforce or check that these data

elements match? At a minimum, the function needs to sanitize the header data
before it writes it out, ensuring that the necessary entries (ie, TCOL, etc) are
written to the header.

Some possible solutions:

  • require validation by the psFitsWriteImage / Table function
  • define a function to strip the psMetadata structure down to only the valid

FITS header entries

note that the table type could be assigned by checking the header for the value
of IMAGETYP (BINTABLE or TABLE). The same is true for the value of EXTNAME.

Alternatively, the function call (with the table type / extname parameter) could
set that value correctly in the header. I think I lean towards this, in which
case we need the table type and extname arguments.

comment:18 by eugene, 21 years ago

Status: newassigned

Robert: Have we converged on this? Can I close this bug as far as you are
concerned?

comment:19 by robert.desonia@…, 21 years ago

Resolution: fixed
Status: assignedclosed

Sure, let's close this. If I come across any concerns against what is currently
in the SDRS, I'll create a new report.

-rdd

Note: See TracTickets for help on using tickets.