MobleyLab / chemper

Repository for Chemical Perception Sampling Tools
MIT License
19 stars 10 forks source link

Demonstrate the problem with `__eq__` on AtomStorage Objects #27

Closed bannanc closed 6 years ago

bannanc commented 6 years ago

This pull request is just a demonstration of what happens when I include a __eq__ function in my AtomStorage objects in either the ChemPerGraphs or ClusterGraphs.

It seems that by overriding the default __eq__ function makes the AtomStorage object unhashable meaning it can't be used as the node in a networkx graph. I'd like to understand better why setting that function makes a class unhashable. Is there a better way to handle the nodes in graphs so a nonhashable object could be stored in them.

Here is a mini example of what happens when you put a custom class in a networkx graph node. First with no __eq__ function:

import networkx as nx

class A(object):
    """
    sand box class
    """
    def __init__(self, letter):
        self.letter = letter

    def __str__(self):
        return str(self.letter)

graph = nx.Graph()
a = A('a')
graph.add_node(a)
print([str(n) for n in graph.nodes()])

prints ['a']

Then when I add an __eq__ function:

import networkx as nx

class A(object):
    """
    sand box class
    """
    def __init__(self, letter):
        self.letter = letter

    def __str__(self):
        return str(self.letter)

    def __eq__(self, other):
        if isinstance(other, self.__class__):
            return self.letter == other.letter
        return False

graph = nx.Graph()
a = A('a')
graph.add_node(a)
print([str(n) for n in graph.nodes()])

at the graph.add_node(a) line I get this output:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-14-9904e8d48bd9> in <module>()
     18 graph = nx.Graph()
     19 a = A('a')
---> 20 graph.add_node(a)
     21 print([str(n) for n in graph.nodes()])

~/miniconda/envs/chemper_testoe/lib/python3.5/site-packages/networkx/classes/graph.py in add_node(self, node_for_adding, **attr)
    479         doesn't change on mutables.
    480         """
--> 481         if node_for_adding not in self._node:
    482             self._adj[node_for_adding] = self.adjlist_inner_dict_factory()
    483             self._node[node_for_adding] = attr

TypeError: unhashable type: 'A'
bannanc commented 6 years ago

Note, I'm intentionally sharing the failing version of this code, otherwise the PR is the same as #24 so there is no need to do code review.

bannanc commented 6 years ago

Daniel and I discussed this a little on Slack, I'm going to go ahead and close this PR, because I think I have the resources for how to set the __eq__ and __hash__ functions if I need to.