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

Python PropertySet.set mis-handles array of bool

    Details

      Description

      PropertySet and PropertyList both mis-handle set(name, array-of-bool). The call succeeds, but the item is not correctly saved. Consider the following example:

      from lsst.daf.base import PropertySet, PropertyList
      ps = PropertySet()  # or PropertyList()
      ps.set("A", [False, True])
      ps.get("A")
      # throws *** lsst.pex.exceptions.wrappers.TypeError: Unknown PropertySet value type for A
      ps.set("B", False)
      ps.add("B", True)
      ps.get("B")
      # returns [False, True]
      

      Note that it is possible to set an array of bool using add, so it seems to be something about PropertySet.set.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment - - edited

            The problem appears to be a bug or misfeature of boost::any. Here is a simple demo program:

            #include <iostream>
            #include <vector>
            #include "boost/any.hpp"
             
            int main() {
                std::vector<bool> value = {false, true};
                // this works
                for (int i = 0; i < value.size(); ++i) {
                    boost::any v1 = static_cast<bool>(value[i]);
                    std::cout << "i=" << i << "; value=" << boost::any_cast<bool>(v1) << std::endl;
                }
                // this does not
                for (int i = 0; i < value.size(); ++i) {
                    boost::any v2 = value[i];
                    std::cout << "i=" << i << "; value=" << boost::any_cast<bool>(v2) << std::endl;
                }
            }
            

            When I build and run this on my Mac I see:

            i=0; value=0
            i=1; value=1
            libc++abi.dylib: terminating with uncaught exception of type boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_any_cast> >: boost::bad_any_cast: failed conversion using boost::any_cast
            i=0; value=Abort trap: 6
            

            A variant of this that uses int instead of bool runs just fine.

            Show
            rowen Russell Owen added a comment - - edited The problem appears to be a bug or misfeature of boost::any . Here is a simple demo program: #include <iostream> #include <vector> #include "boost/any.hpp"   int main() { std::vector<bool> value = {false, true}; // this works for (int i = 0; i < value.size(); ++i) { boost::any v1 = static_cast<bool>(value[i]); std::cout << "i=" << i << "; value=" << boost::any_cast<bool>(v1) << std::endl; } // this does not for (int i = 0; i < value.size(); ++i) { boost::any v2 = value[i]; std::cout << "i=" << i << "; value=" << boost::any_cast<bool>(v2) << std::endl; } } When I build and run this on my Mac I see: i=0; value=0 i=1; value=1 libc++abi.dylib: terminating with uncaught exception of type boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_any_cast> >: boost::bad_any_cast: failed conversion using boost::any_cast i=0; value=Abort trap: 6 A variant of this that uses int instead of bool runs just fine.
            Hide
            rowen Russell Owen added a comment -

            I fixed the problem by using dest.push_back(static_cast<T>(val)); to append values, in a new private function _append (the underscore is intended to make it clear that this is not a public function; I can use a longer name without an underscore if you prefer).

            I also fixed the compiler warnings in DM-13220 with another commit. The solution has a bit more duplication than I like, but it is simple and readable and I could not find a way to use std::enable_if.

            I also modernized the C++ code in PropertySet.cc and PropertyList.cc and took another clang-format pass on all the C++.

            Show
            rowen Russell Owen added a comment - I fixed the problem by using dest.push_back(static_cast<T>(val)); to append values, in a new private function _append (the underscore is intended to make it clear that this is not a public function; I can use a longer name without an underscore if you prefer). I also fixed the compiler warnings in DM-13220 with another commit. The solution has a bit more duplication than I like, but it is simple and readable and I could not find a way to use std::enable_if . I also modernized the C++ code in PropertySet.cc and PropertyList.cc and took another clang-format pass on all the C++.
            Hide
            ktl Kian-Tat Lim added a comment -

            Looks good.  One (long-standing) license typo and one question, but OK to merge.

            Show
            ktl Kian-Tat Lim added a comment - Looks good.  One (long-standing) license typo and one question, but OK to merge.
            Hide
            rowen Russell Owen added a comment -

            Thank you for the helpful review. I fixed the items you found.

            Show
            rowen Russell Owen added a comment - Thank you for the helpful review. I fixed the items you found.

              People

              • Assignee:
                rowen Russell Owen
                Reporter:
                rowen Russell Owen
                Reviewers:
                Kian-Tat Lim
                Watchers:
                Kian-Tat Lim, Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel