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

Make some afw types hashable

    Details

    • Story Points:
      9
    • Sprint:
      AP F18-4, AP F18-5, AP F18-6
    • Team:
      Alert Production

      Description

      The C++11 containers unordered_set and unordered_map only work with hashable types, a concept that was itself added in C++11. Most afw value types were implemented before we adopted C++11, and therefore do not have a specialization of std::hash. Implementing std::hash will also make it easier to implement DM-7621, as the C++11 requirements for hash are stricter than the Python requirements.

      In most cases, hash can be quickly implemented by adopting a modified version of the algorithm from Effective Java, Item 9; the only difference between the C++ requirements and the Java ones is that hash functions are implied to be strictly deterministic before C++14, but the EffJ algorithm meets that criterion if the hash for all member variables does.

      The classes covered by this ticket are a subset (possibly taken from DM-9936) of the following types:

      • afw::cameraGeom::CameraPoint
      • afw::cameraGeom::CameraSys
      • afw::cameraGeom::CameraSysPrefix
      • afw::cameraGeom::Orientation
      • afw::coord::Coord and its subclasses
      • afw::coord::Observatory
      • afw::coord::Weather
      • afw::detection::Threshold
      • afw::geom::Angle
      • afw::geom::AngleUnit
      • afw::geom::Box
      • afw::geom::CoordinateBase and its subclasses
      • afw::geom::CoordinateExpr
      • afw::geom::Span
      • afw::geom::SpherePoint
      • afw::geom::polygon::Polygon
      • afw::image::Calib
      • afw::image::Color
      • afw::image::DefectBase
      • afw::image::Filter
      • afw::image::FilterProperty
      • afw::math::FitResults
      • afw::math::Function and its subclasses
      • afw::math::MaskedVector
      • afw::math::Statistics
      • afw::table::BaseRecord and its subclasses
      • afw::table::ConstFunctorKey
      • afw::table::FieldBase and its subclasses
      • afw::table::InputFunctorKey and its subclasses
      • afw::table::KeyBase and its subclasses
      • afw::table::Match
      • afw::table::OutputFunctorKey and its subclasses
      • afw::table::ReferenceFunctorKey
      • afw::table::SchemaItem
      • afw::table::io::Persistable
      • afw::table::io::PersistableFacade

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            The documentation for afw::cameraGeom has requested that CameraSys be hashable in C++.

            Show
            krzys Krzysztof Findeisen added a comment - The documentation for afw::cameraGeom has requested that CameraSys be hashable in C++.
            Hide
            jbosch Jim Bosch added a comment -

            Shouldn't Python types only be hashable if they are also immutable?  (I don't know how serious the problems are if you use a hashable-but-mutable object as a key in a dict and then mutate it.)

            Show
            jbosch Jim Bosch added a comment - Shouldn't Python types only be hashable if they are also immutable?  (I don't know how serious the problems are if you use a hashable-but-mutable object as a key in a dict and then mutate it.)
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            I'll reply on DM-7621, since DM-9938 only covers making things hashable in C++.

            Show
            krzys Krzysztof Findeisen added a comment - - edited I'll reply on DM-7621 , since DM-9938 only covers making things hashable in C++.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            Hi Jim Bosch, would you be willing to review this? It's nominally 1400 lines, but there's a lot of boilerplate (e.g., new copyright headers alone account for about 500 lines). Note that this is a C++-only change; python/pybind11 changes changes are out of scope (DM-7621).

            afw PR is not showing up, as usual; it's #410.

            Show
            krzys Krzysztof Findeisen added a comment - - edited Hi Jim Bosch , would you be willing to review this? It's nominally 1400 lines, but there's a lot of boilerplate (e.g., new copyright headers alone account for about 500 lines). Note that this is a C++-only change; python/pybind11 changes changes are out of scope ( DM-7621 ). afw PR is not showing up, as usual; it's #410 .
            Hide
            jbosch Jim Bosch added a comment -

            Sure.  Probably won't be able to look at it until Wednesday, and please remind me if I haven't completed the review by Friday.

            Show
            jbosch Jim Bosch added a comment - Sure.  Probably won't be able to look at it until Wednesday, and please remind me if I haven't completed the review by Friday.
            Hide
            jbosch Jim Bosch added a comment -

            Great work.  Just some superficial comments on the PRs.

            Show
            jbosch Jim Bosch added a comment - Great work.  Just some superficial comments on the PRs.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Thanks for the review!

            Show
            krzys Krzysztof Findeisen added a comment - Thanks for the review!

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Jim Bosch
                Watchers:
                Jim Bosch, Krzysztof Findeisen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel