Distinct instances of the same node do not compare equal

[borellim]

Example input:

n1 = load_node(42)
n2 = load_node(42)
print n1, type(n1)
print n2, type(n2)
print n1 == n2

Output:

uuid: ddf9a6de-56bf-4783-96d2-33400df20f43 (pk: 42) <class 'aiida.orm.data.upf.UpfData'>
uuid: ddf9a6de-56bf-4783-96d2-33400df20f43 (pk: 42) <class 'aiida.orm.data.upf.UpfData'>
False

Tested with types UpfData, WorkCalculation, PwCalculation.

EDIT: also tested with more "basic" types aiida.orm.data.str.Str and aiida.orm.data.int.Int. For these, some more discussion is possibly required: should the PK/UUID be compared, or the actual data themselves?

[ltalirz]

@sphuber You mention here that there may be reasons for not implementing the behaviour asked for in this issue.
Would be curious to know what these reasons were ... perhaps @giovannipizzi can also comment

[giovannipizzi]

Mmm not sure.
I feel that we should not compare the content (otherwise the behaviour depends on the data type).

However, I cannot think now to a real problem why implementing comparison by PK (or even better by UUID - in case in the future we support loading at the same time multiple profiles, and different nodes might have the same PK).

I suggest to implement it unless we realise there's a problem.

[sphuber]

Mmm not sure.
I feel that we should not compare the content (otherwise the behaviour depends on the data type).

I think this would be mostly very difficult to implement correctly and be sure it always behaves correctly.

However, I cannot think now to a real problem why implementing comparison by PK (or even better by UUID.

Since the UUID is what we consider to uniquely identify a node in our data model, I think it is OK to base the equality operation on this, so I am also in favor implementing this.

I suggest to implement it unless we realise there's a problem.

The only thing I can think of is where someone in code may change the UUID of an unstored node to an existing one manually and then do a comparison, but I don't think this should pose a problem. This example is outlandish and storing the node would fail now that there is a uniqueness constraint on the UUID column.

[ramirezfranciscof]

@ltalirz @giovannipizzi @sphuber One thing I just discovered with this, is that currently there are some data nodes that use a different criteria to establish the concept of equality. I don't know the full extent of this, but I think this originates from the BaseType inside of orm/nodes/data/base.py, so it should apply to all nodes that mock python data types. For example the Int type will check if the value that is stored is equal.

This now poses a series of problems:

1. Changing the existing behaviour would break backwards compatibility.
2. Having both behaviors depending on the type of node would lack consistency.
3. Using value as criteria in python-like nodes allows for an intuitive/natural handling of those nodes but can't be extended to other node types.
4. Using uuid as criteria is extensible and consistent, but we would loose the "natural" handling of these basic node types.

I'm unsure on how to proceed. I am inclined towards going for uuid in all cases and just biting the bullet on 1 and 4, but I wanted to hear what you guys think. Maybe we should discuss this in the next aiida meeting...

[sphuber]

Very good catch. This does indeed complicate things. I definitely agree that we should have one or the other and not a mix of behavior that depends on the type. However, this would lead us naturally to only ever allowing to do comparison based on UUIDs. This would mean we just need to deprecate the current implementations of __eq__ and say that as of v2.0.0 this will work differently and only compare UUIDs. But I agree that this would be good to discuss in a meeting.

[greschd]

Yeah, I have to say I'm quite opposed to changing the equality behavior of the numeric types. That can break existing code in quite complicated and hard-to-find cases.

What's the harm in comparing by UUID just as a fallback, for the types that don't have a custom comparison function? After all, that is e.g. how the built-in Python types do it (compare by value, and otherwise by id()).