Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-20506

Allow PropertySet to handle unsigned 64-bit integers

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • daf_base
    • External

    Description

      For writing PFS data, we want elements of the dataId to allow unsigned 64-bit integer values (e.g., object identifiers or hashes). This is not currently possible, because the butler puts the dataId into a PropertySet (as "additionalData"), and PropertySet does not support unsigned 64-bit integers (signed 64-bit integers are allowed).

      I will add u64 support to PropertySet. This may not be something that LSST cares strongly about (because of Gen3 efforts), but this is something that we would like to see get into a 18.x release so we can base the PFS work off that for the next year or so.

      Attachments

        Issue Links

          Activity

            price Paul Price added a comment -

            tjenness, would you mind having a look at this change, since I believe you're the last person to consider this area.

            A Jenkins run is underway.

            price@price-laptop:~/LSST/daf_base/python (tickets/DM-20506=) $ git sub
            commit a0d3076fddd8a52db762548341633fbe1ce28f0c (HEAD -> tickets/DM-20506, origin/tickets/DM-20506)
            Author: Paul Price <price@astro.princeton.edu>
            Date:   Wed Jul 10 13:53:31 2019 -0400
             
                PropertySet: add support for u64
                
                This allows the use of unsigned 64-bit integers (e.g., for
                object IDs or hashes) as values in PropertySet. This can be
                important, since the Gen2 butler stuffs the dataId into a
                PropertySet.
             
             include/lsst/daf/base/PropertySet.h                | 11 +++++++++
             .../propertyContainerContinued.py                  | 18 +++++++++++----
             .../lsst/daf/base/propertyContainer/propertySet.cc |  2 ++
             src/PropertySet.cc                                 | 27 ++++++++++++++++++++++
             tests/test_PropertySet_1.cc                        | 15 ++++++++++++
             tests/test_PropertySet_2.py                        | 25 ++++++++++++++++++++
             6 files changed, 94 insertions(+), 4 deletions(-)
            

            price Paul Price added a comment - tjenness , would you mind having a look at this change, since I believe you're the last person to consider this area. A Jenkins run is underway. price@price-laptop:~/LSST/daf_base/python (tickets/DM-20506=) $ git sub commit a0d3076fddd8a52db762548341633fbe1ce28f0c (HEAD -> tickets/DM-20506, origin/tickets/DM-20506) Author: Paul Price <price@astro.princeton.edu> Date: Wed Jul 10 13:53:31 2019 -0400   PropertySet: add support for u64 This allows the use of unsigned 64-bit integers (e.g., for object IDs or hashes) as values in PropertySet. This can be important, since the Gen2 butler stuffs the dataId into a PropertySet.   include/lsst/daf/base/PropertySet.h | 11 +++++++++ .../propertyContainerContinued.py | 18 +++++++++++---- .../lsst/daf/base/propertyContainer/propertySet.cc | 2 ++ src/PropertySet.cc | 27 ++++++++++++++++++++++ tests/test_PropertySet_1.cc | 15 ++++++++++++ tests/test_PropertySet_2.py | 25 ++++++++++++++++++++ 6 files changed, 94 insertions(+), 4 deletions(-)
            tjenness Tim Jenness added a comment -

            Looks okay to me. Is there a test for trying to set an usigned long long using a negative int?

            tjenness Tim Jenness added a comment - Looks okay to me. Is there a test for trying to set an usigned long long using a negative int?
            price Paul Price added a comment -

            I just added an explicit test for the various integer ranges:

                def testIntegerRanges(self):
                    """Test that the ranges of the various integer types is as expected"""
                    ps = dafBase.PropertySet()
                    minI32 = -2**31
                    maxI32 = 2**31 - 1
                    minI64 = -2**63
                    maxI64 = 2**63 - 1
                    minU64 = 0
                    maxU64 = 2**64 - 1
                    # Out of range for the particular type
                    self.assertRaises(TypeError, ps.addInt, "int32", minI32 - 1)
                    self.assertRaises(TypeError, ps.addInt, "int32", maxI32 + 1)
                    self.assertRaises(TypeError, ps.addLongLong, "int64", minI64 - 1)
                    self.assertRaises(TypeError, ps.addLongLong, "int64", maxI64 + 1)
                    self.assertRaises(TypeError, ps.addUnsignedLongLong, "uint64", minU64 - 1)
                    self.assertRaises(TypeError, ps.addUnsignedLongLong, "uint64", maxU64 + 1)
                    # Out of all possible integer ranges
                    self.assertRaises(RuntimeError, ps.add, "number", minI64 - 1)
                    self.assertRaises(RuntimeError, ps.add, "number", maxU64 + 1)
            

            price Paul Price added a comment - I just added an explicit test for the various integer ranges: def testIntegerRanges(self): """Test that the ranges of the various integer types is as expected""" ps = dafBase.PropertySet() minI32 = -2**31 maxI32 = 2**31 - 1 minI64 = -2**63 maxI64 = 2**63 - 1 minU64 = 0 maxU64 = 2**64 - 1 # Out of range for the particular type self.assertRaises(TypeError, ps.addInt, "int32", minI32 - 1) self.assertRaises(TypeError, ps.addInt, "int32", maxI32 + 1) self.assertRaises(TypeError, ps.addLongLong, "int64", minI64 - 1) self.assertRaises(TypeError, ps.addLongLong, "int64", maxI64 + 1) self.assertRaises(TypeError, ps.addUnsignedLongLong, "uint64", minU64 - 1) self.assertRaises(TypeError, ps.addUnsignedLongLong, "uint64", maxU64 + 1) # Out of all possible integer ranges self.assertRaises(RuntimeError, ps.add, "number", minI64 - 1) self.assertRaises(RuntimeError, ps.add, "number", maxU64 + 1)
            price Paul Price added a comment -

            New Jenkins run succeeded.

            price Paul Price added a comment - New Jenkins run succeeded.

            Hi price — do you have an ETA on merging this? We're ready to get 18.1.0 done...

            swinbank John Swinbank added a comment - Hi price — do you have an ETA on merging this? We're ready to get 18.1.0 done...
            price Paul Price added a comment -

            ETA is -30 seconds.

            Merged to master.

            price Paul Price added a comment - ETA is -30 seconds. Merged to master.

            People

              price Paul Price
              price Paul Price
              Tim Jenness
              John Swinbank, Paul Price, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.