Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce equality concept for nodes + tests #4352

Closed

Conversation

ramirezfranciscof
Copy link
Member

@ramirezfranciscof ramirezfranciscof commented Sep 3, 2020

Two nodes will evaluate to be equal if their uuid coincides.

Fixes #1917

Two nodes will evaluate to be equal if their uuid coincides.
@ramirezfranciscof ramirezfranciscof marked this pull request as draft September 3, 2020 13:08
@ramirezfranciscof
Copy link
Member Author

This is now WIP and paused, see this comment in issue #1917

def test_equality(self):
"""Test the `__eq__` method."""
node1 = Node()
node2 = node1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't work quite as intended: Because node1 and node2 are the same Python object (id(node1) == id(node2)), node1 == node2 will be True even in the current code.

Instead, I think we need to store a Data node and load it back, such that node1 and node2 are different Python objects.

That can be tested by assert node1 is not node2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the following (current behavior, before the applied fix):

In [1]: from aiida.orm import Data

In [2]: d1 = Data().store()

In [3]: d2 = d1

In [4]: d3 = load_node(d1.uuid)

In [5]: d1 == d2
Out[5]: True

In [6]: d1 is d2
Out[6]: True

In [7]: d1 == d3
Out[7]: False

In [8]: d1 is d3
Out[8]: False

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I had a slight suspicion of this but I found the problems with the intrinsic python-mock node types before I could get to that. I would anyways have had to ask you guys how to check this for sure, haha.

@ramirezfranciscof
Copy link
Member Author

After some discussion in the original issue #1917 I will be closing this down until we reach a final agreement and then I can implement a more thorough and complete solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distinct instances of the same node do not compare equal
2 participants