Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-239

PropertyList should support missing values and possibly units

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      PropertyList is how we perform FITS header I/O, but it does not support missing values, which are used in FITS headers to indicate items that have unknown value.

      Furthermore, it would be nice to have support for units as a separate field, though for FITS headers one must include that information as part of the comment.

      I am not necessarily proposing to do this work, but I felt I could at least get permission for it to be done.

        Attachments

          Issue Links

            Activity

            No builds found.
            rowen Russell Owen created issue -
            rowen Russell Owen made changes -
            Field Original Value New Value
            Watchers Russell Owen [ Russell Owen ] John Parejko, Russell Owen, Tim Jenness [ John Parejko, Russell Owen, Tim Jenness ]
            Hide
            Parejkoj John Parejko added a comment -

            I support this, even if we don't use FITS for internal data persistence (as I'm hoping will happen before too long): being able to attach comments and units to PropertySet values is a very useful documentation feature (I'm right this moment lamenting the ability to add docstrings to collection.namedtuple entries).

            Show
            Parejkoj John Parejko added a comment - I support this, even if we don't use FITS for internal data persistence (as I'm hoping will happen before too long): being able to attach comments and units to PropertySet values is a very useful documentation feature (I'm right this moment lamenting the ability to add docstrings to collection.namedtuple entries).
            Hide
            tjenness Tim Jenness added a comment -

            I'm strongly in favor of adding support for undefined values. Reading (and writing) units from comments would also be very useful. I'm a bit surprised that comments aren't handled. How is round-tripping done?

            Show
            tjenness Tim Jenness added a comment - I'm strongly in favor of adding support for undefined values. Reading (and writing) units from comments would also be very useful. I'm a bit surprised that comments aren't handled. How is round-tripping done?
            Hide
            ktl Kian-Tat Lim added a comment -

            PropertyList does support comments. There are comment arguments to all modifier methods, a getComment() method is present, and comments are output after "// " in the toString() method.

            Is it better to have unit support in PropertyList or is it better to store objects with units in a PropertyList? I was anticipating the latter.

            Adding missing value and unit support seems likely to take PropertyList further from being a Python OrderedDict (and PropertySet further from being a dict).

            Show
            ktl Kian-Tat Lim added a comment - PropertyList does support comments. There are comment arguments to all modifier methods, a getComment() method is present, and comments are output after "// " in the toString() method. Is it better to have unit support in PropertyList or is it better to store objects with units in a PropertyList ? I was anticipating the latter. Adding missing value and unit support seems likely to take PropertyList further from being a Python OrderedDict (and PropertySet further from being a dict ).
            Hide
            price Paul Price added a comment -

            While you're thinking about this, do we support reading a 'NaN' value in the header as a floating-point value?

            Show
            price Paul Price added a comment - While you're thinking about this, do we support reading a 'NaN' value in the header as a floating-point value?
            Hide
            rowen Russell Owen added a comment -

            Paul Price we do support writing and reading NaN to FITS headers (since PropertyList floats can be Nan), but it is not a FITS-compliant thing to do and FITS checkers complain about it. I'd rather not go down that route if we can avoid it.

            Show
            rowen Russell Owen added a comment - Paul Price we do support writing and reading NaN to FITS headers (since PropertyList floats can be Nan), but it is not a FITS-compliant thing to do and FITS checkers complain about it. I'd rather not go down that route if we can avoid it.
            rowen Russell Owen made changes -
            Summary PropertyList should support comments and missing values PropertyList should missing values and possibly units
            rowen Russell Owen made changes -
            Description PropertyList is how we perform FITS header I/O, but it is missing two important features for FITS files:
            - It does not support comments
            - It does not support missing values (which are in FITS headers used to indicate items that have unknown value).

            Furthermore, it would be nice to have support for units.

            I further propose that PropertySet gain support for comments and units (and missing values if practical, to keep the differences between PropertyList and PropertySet small).

            I am not necessarily proposing to do this work, but I felt I could at least get permission for it to be done.
            PropertyList is how we perform FITS header I/O, but it does not support missing values, which are used in FITS headers to indicate items that have unknown value.

            Furthermore, it would be nice to have support for units as a separate field, though for FITS headers one must include that information as part of the comment.

            I am not necessarily proposing to do this work, but I felt I could at least get permission for it to be done.
            Hide
            rowen Russell Owen added a comment -

            Kian-Tat Lim I apologize for misrepresenting comment support in PropertyList. I don't know how I managed to forget that. I updated the title and description accordingly.

            If we are going to support FITS properly I think support for missing fields is necessary. The trigger is VisitInfo which is often missing some values.

            Show
            rowen Russell Owen added a comment - Kian-Tat Lim I apologize for misrepresenting comment support in PropertyList . I don't know how I managed to forget that. I updated the title and description accordingly. If we are going to support FITS properly I think support for missing fields is necessary. The trigger is VisitInfo which is often missing some values.
            Hide
            tjenness Tim Jenness added a comment -

            Paul Price are you talking about:

            KEYWORD =       NaN / Floating point that came out wrong
            

            or

            KEYWORD =     'NaN' / not a number written out as a string
            

            ?

            Show
            tjenness Tim Jenness added a comment - Paul Price are you talking about: KEYWORD = NaN / Floating point that came out wrong or KEYWORD = 'NaN' / not a number written out as a string ?
            Hide
            price Paul Price added a comment -

            I think we want to be able to round-trip floating-point values, so we need to support NaN and +/- Inf. I don't think FITS supports these natively, so you have to write them as strings.

            Show
            price Paul Price added a comment - I think we want to be able to round-trip floating-point values, so we need to support NaN and +/- Inf . I don't think FITS supports these natively, so you have to write them as strings.
            Hide
            Parejkoj John Parejko added a comment -

            How does writing NAN/Inf as strings help? We would have to do some strange parsing internally, and they would break everything else externally. Plus, strings wouldn't even work for some keywords that must be a certain type.

            Show
            Parejkoj John Parejko added a comment - How does writing NAN/Inf as strings help? We would have to do some strange parsing internally, and they would break everything else externally. Plus, strings wouldn't even work for some keywords that must be a certain type.
            Hide
            price Paul Price added a comment -

            I don't want to hijack this thread, so let's take the NaN issue offline.

            Show
            price Paul Price added a comment - I don't want to hijack this thread, so let's take the NaN issue offline.
            rowen Russell Owen made changes -
            Summary PropertyList should missing values and possibly units PropertyList should support missing values and possibly units
            Hide
            rowen Russell Owen added a comment -

            This RFC is accepted as follows:

            • PropertyList will be extended to support missing (unknown) values, in a way that is compatible with the FITS standard for header cards.
            • PropertySet need not not gain same support, but if it can be added without doing violence to the class, then it would help keep the classes more closely aligned.
            • Consider using None as the value for missing values in Python.
            • How important this is, and whether we can safely rely on it, depends partly on how pyfits handles headers with missing values. We know pyfits cannot write such headers, but we don't know what happens when it tries to read them.
            • Do not add support for units as part of this RFC. Feel free to make a new RFC for that.
            Show
            rowen Russell Owen added a comment - This RFC is accepted as follows: PropertyList will be extended to support missing (unknown) values, in a way that is compatible with the FITS standard for header cards. PropertySet need not not gain same support, but if it can be added without doing violence to the class, then it would help keep the classes more closely aligned. Consider using None as the value for missing values in Python. How important this is, and whether we can safely rely on it, depends partly on how pyfits handles headers with missing values. We know pyfits cannot write such headers, but we don't know what happens when it tries to read them. Do not add support for units as part of this RFC. Feel free to make a new RFC for that.
            rowen Russell Owen made changes -
            Link This issue is triggering DM-8100 [ DM-8100 ]
            rowen Russell Owen made changes -
            Link This issue is triggering DM-8101 [ DM-8101 ]
            rowen Russell Owen made changes -
            Resolution Done [ 10000 ]
            Status Proposed [ 10805 ] Adopted [ 10806 ]
            tjenness Tim Jenness made changes -
            Link This issue is triggering DM-9873 [ DM-9873 ]
            tjenness Tim Jenness made changes -
            Status Adopted [ 10806 ] Implemented [ 11105 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20404 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20444 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20444 ]

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Watchers:
              John Parejko, Kian-Tat Lim, Paul Price, Russell Owen, Tim Jenness, Xiuqin Wu [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.