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

Immutable afw objects should have __hashable__

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Won't Fix
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw

      Description

      Immutable types with python interfaces should define a __hash__ method so that they can be used as dictionary keys and in sets in python3.

      For example, trying to build a set out of Angles results in the following error:

      >   ras = {s.getRa() for s in cat}
      E   TypeError: unhashable type: 'Angle'
      

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            My vision of the right way to approach this is to wrap small mutable C++ objects as immutable Python objects - the difference in the behavior of the assignment operator (and augmented assignment operators) makes this much more natural than you might think.  You also have to make sure you pass by value or const reference across the C++/Python border, but that's probably already good practice for the (mostly small) objects one would typically want to make hashable and immutable.

            That's of course not an answer if we don't want to break all of our low-level APIs by making many more objects immutable.  I don't really have a good answer for that, though it may be helpful to do some experiments to see what actually happens if you give a Python set or dict a mutable hashable and then modify it.  If the result is "an exception", I'd probably agree with adding hash support to at least some mutable object.  If the result is a segfault, infinite loops, or silently incorrect results, I'd be more cautious.

            Show
            jbosch Jim Bosch added a comment - My vision of the right way to approach this is to wrap small mutable C++ objects as immutable Python objects - the difference in the behavior of the assignment operator (and augmented assignment operators) makes this much more natural than you might think.  You also have to make sure you pass by value or const reference across the C++/Python border, but that's probably already good practice for the (mostly small) objects one would typically want to make hashable and immutable. That's of course not an answer if we don't want to break all of our low-level APIs by making many more objects immutable.  I don't really have a good answer for that, though it may be helpful to do some experiments to see what actually happens if you give a Python set or dict a mutable hashable and then modify it.  If the result is "an exception", I'd probably agree with adding hash support to at least some mutable object.  If the result is a segfault, infinite loops, or silently incorrect results, I'd be more cautious.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            From the following snippet I conclude that the answer is "silently incorrect results":

            In [1]: class X:
               ...:     def __init__(self, x):
               ...:         self.x = x
               ...: 
               ...:     def __hash__(self):
               ...:         return self.x
               ...: 
               ...:     def __repr__(self):
               ...:         return str(self.x)
               ...: 
               ...: a = X(1)
               ...: b = X(2)
               ...: 
               ...: s = set((a, b))
               ...: 
               ...: print(s)
               ...: print(a in s)
               ...: 
               ...: a.x = 5
               ...: 
               ...: print(s)
               ...: print(a in s)
               ...: 
               ...: 
            {1, 2}
            True
            {5, 2}
            False
            

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - From the following snippet I conclude that the answer is "silently incorrect results": In [ 1 ]: class X: ...: def __init__( self , x): ...: self .x = x ...: ...: def __hash__( self ): ...: return self .x ...: ...: def __repr__( self ): ...: return str ( self .x) ...: ...: a = X( 1 ) ...: b = X( 2 ) ...: ...: s = set ((a, b)) ...: ...: print (s) ...: print (a in s) ...: ...: a.x = 5 ...: ...: print (s) ...: print (a in s) ...: ...: { 1 , 2 } True { 5 , 2 } False
            Hide
            tjenness Tim Jenness added a comment -

            That's what I expected. I think Python trusts that you aren't going to change the object from underneath it.

            Show
            tjenness Tim Jenness added a comment - That's what I expected. I think Python trusts that you aren't going to change the object from underneath it.
            Hide
            tjenness Tim Jenness added a comment -

            How much of this work was done in DM-9938?

            Show
            tjenness Tim Jenness added a comment - How much of this work was done in DM-9938 ?
            Hide
            jbosch Jim Bosch added a comment -

            DM-9938 looks quite thorough to me, and I don't think keeping this ticket around for any unidentified stragglers is worth it. Closing as Won't Fix.

            Show
            jbosch Jim Bosch added a comment - DM-9938 looks quite thorough to me, and I don't think keeping this ticket around for any unidentified stragglers is worth it. Closing as Won't Fix.

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              Parejkoj John Parejko
              Watchers:
              Jim Bosch, John Parejko, Pim Schellart [X] (Inactive), Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.