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

Allow PropertySet to handle unsigned 64-bit integers

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_base
    • Labels:
    • Team:
      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

            Hide
            price Paul Price added a comment -

            Tim Jenness, 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(-)
            

            Show
            price Paul Price added a comment - Tim Jenness , 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(-)
            Hide
            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?

            Show
            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?
            Hide
            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)
            

            Show
            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)
            Hide
            price Paul Price added a comment -

            New Jenkins run succeeded.

            Show
            price Paul Price added a comment - New Jenkins run succeeded.
            Hide
            swinbank John Swinbank added a comment -

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

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

            ETA is -30 seconds.

            Merged to master.

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

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel