#226 closed defect (fixed)
FITS API suggested changes.
| Reported by: | 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 , 22 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:2 by , 22 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
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 , 22 years ago
| Owner: | changed from to |
|---|---|
| Status: | reopened → new |
comment:4 by , 22 years ago
| Status: | new → assigned |
|---|
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 , 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 , 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 , 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 , 22 years ago
I've cleaned the wording in the SDRS to reflect the addition of the 'depth'
argument.
comment:9 by , 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 , 22 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
comment:11 by , 22 years ago
| Status: | new → assigned |
|---|
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 , 22 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
comment:13 by , 22 years ago
| Owner: | changed from to |
|---|
comment:14 by , 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 , 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 , 22 years ago
For psFitsWriteImage / psFitsWriteTable, should there not be another parameter
giving the extension name of the HDU being created?
comment:17 by , 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 , 21 years ago
| Status: | new → assigned |
|---|
Robert: Have we converged on this? Can I close this bug as far as you are
concerned?
comment:19 by , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
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

I've modified the current draft of version 09 of the SDRS to match the listing
below, with a few minor mods:
of psMetadata collections)
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.