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

[FIX] Fix variables equality and hashes #4957

Merged
merged 2 commits into from
Aug 28, 2020

Conversation

PrimozGodec
Copy link
Contributor

@PrimozGodec PrimozGodec commented Aug 27, 2020

Issue

Fixes (partially or completely) #4870

Problem is the following:

from Orange.data import Domain, DiscreteVariable
a = DiscreteVariable("Cluster", values=["c"])
var1 = a.renamed("Cluster x")
var2 = DiscreteVariable("Cluster", values=["a", "b"])
domain = Domain([], metas=[var1, var2])

print(var1 == var2)  # vars have completely different origin but still equal
>>> True
print(domain[var1]) # wrong attribute because of wrong comparison -> should return Cluster x
>>> Cluster

The problem is that var1 and var2 have equal hash (also __eq__ returns True). This happens since var1 have compute_value with identity pointing to the variable a; variables a and var2 have same hash since they have the same name and same compute_value (None).

The Domain has a dictionary with index for each variable. Variables are keys in dict, when updating dictionary index is set for variable Cluster x instead of Cluster (same hashes).

Description of changes

Variables are not equal if their names are not equal and their hashes are not equal either. This avoids problems in the domain (two vars with same hash).

Includes
  • Code changes
  • Tests
  • Documentation

@PrimozGodec PrimozGodec added the needs discussion Core developers need to discuss the issue label Aug 27, 2020
@PrimozGodec PrimozGodec changed the title [FIX] Domain: fix getitem [WIP][FIX] Domain: fix getitem Aug 27, 2020
@PrimozGodec PrimozGodec force-pushed the fix-merge-data branch 3 times, most recently from 798316a to 84ec585 Compare August 28, 2020 09:06
@PrimozGodec PrimozGodec changed the title [WIP][FIX] Domain: fix getitem [WIP][FIX] Fix variables equality and hashes Aug 28, 2020
@PrimozGodec PrimozGodec removed the needs discussion Core developers need to discuss the issue label Aug 28, 2020
@PrimozGodec PrimozGodec changed the title [WIP][FIX] Fix variables equality and hashes [FIX] Fix variables equality and hashes Aug 28, 2020
@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

Merging #4957 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4957   +/-   ##
=======================================
  Coverage   84.28%   84.28%           
=======================================
  Files         283      283           
  Lines       58016    58016           
=======================================
+ Hits        48898    48900    +2     
+ Misses       9118     9116    -2     

@janezd janezd merged commit eb3d6ec into biolab:master Aug 28, 2020
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.

2 participants