Merge pull request #4425 from janezd/owconcatenate-merge

[FIX] Concatenate: Fix wrong merging of categorical features

Orange/data/variable.py

@@ -460,6 +460,10 @@ def copy(self, compute_value=None, *, name=None, **kwargs):
         var.attributes = dict(self.attributes)
         return var
 
+    def renamed(self, new_name):
+        # prevent cyclic import, pylint: disable=import-outside-toplevel
+        from Orange.preprocess.transformation import Identity
+        return self.copy(name=new_name, compute_value=Identity(variable=self))
 
 del _predicatedescriptor
 
@@ -552,11 +556,16 @@ def repr_val(self, val):
     str_val = repr_val
 
     def copy(self, compute_value=None, *, name=None, **kwargs):
-        var = super().copy(compute_value=compute_value, name=name,
-                           number_of_decimals=self.number_of_decimals,
-                           **kwargs)
-        var.adjust_decimals = self.adjust_decimals
-        var.format_str = self._format_str
+        # pylint understand not that `var` is `DiscreteVariable`:
+        # pylint: disable=protected-access
+        number_of_decimals = kwargs.pop("number_of_decimals", None)
+        var = super().copy(compute_value=compute_value, name=name, **kwargs)
+        if number_of_decimals is not None:
+            var.number_of_decimals = number_of_decimals
+        else:
+            var._number_of_decimals = self._number_of_decimals
+            var.adjust_decimals = self.adjust_decimals
+            var.format_str = self._format_str
         return var
 
 

Orange/widgets/data/owconcatenate.py

@@ -6,14 +6,17 @@
 
 """
 
-from collections import OrderedDict
+from collections import OrderedDict, namedtuple
 from functools import reduce
+from itertools import chain, count
+from typing import List
 
 import numpy as np
 from AnyQt.QtWidgets import QFormLayout
 from AnyQt.QtCore import Qt
 
 import Orange.data
+from Orange.data.util import get_unique_names_duplicates, get_unique_names
 from Orange.util import flatten
 from Orange.widgets import widget, gui, settings
 from Orange.widgets.settings import Setting
@@ -43,6 +46,10 @@ class Outputs:
     class Error(widget.OWWidget.Error):
         bow_concatenation = Msg("Inputs must be of the same type.")
 
+    class Warning(widget.OWWidget.Warning):
+        renamed_variables = Msg(
+            "Variables with duplicated names have been renamed.")
+
     merge_type: int
     append_source_column: bool
     source_column_role: int
@@ -172,18 +179,15 @@ def incompatible_types(self):
         return False
 
     def apply(self):
+        self.Warning.renamed_variables.clear()
         tables, domain, source_var = [], None, None
         if self.primary_data is not None:
             tables = [self.primary_data] + list(self.more_data.values())
             domain = self.primary_data.domain
         elif self.more_data:
             tables = self.more_data.values()
-            if self.merge_type == OWConcatenate.MergeUnion:
-                domain = reduce(domain_union,
-                                (table.domain for table in tables))
-            else:
-                domain = reduce(domain_intersection,
-                                (table.domain for table in tables))
+            domains = [table.domain for table in tables]
+            domain = self.merge_domains(domains)
 
         if tables and self.append_source_column:
             assert domain is not None
@@ -192,7 +196,7 @@ def apply(self):
                 names = ['{} ({})'.format(name, i)
                          for i, name in enumerate(names)]
             source_var = Orange.data.DiscreteVariable(
-                self.source_attr_name,
+                get_unique_names(domain, self.source_attr_name),
                 values=names
             )
             places = ["class_vars", "attributes", "metas"]
@@ -236,36 +240,76 @@ def send_report(self):
                 self.id_roles[self.source_column_role].lower())
         self.report_items(items)
 
-
-def unique(seq):
-    seen_set = set()
-    for el in seq:
-        if el not in seen_set:
-            yield el
-            seen_set.add(el)
-
-
-def domain_union(a, b):
-    union = Orange.data.Domain(
-        tuple(unique(a.attributes + b.attributes)),
-        tuple(unique(a.class_vars + b.class_vars)),
-        tuple(unique(a.metas + b.metas))
-    )
-    return union
-
-
-def domain_intersection(a, b):
-    def tuple_intersection(t1, t2):
-        inters = set(t1) & set(t2)
-        return tuple(unique(el for el in t1 + t2 if el in inters))
-
-    intersection = Orange.data.Domain(
-        tuple_intersection(a.attributes, b.attributes),
-        tuple_intersection(a.class_vars, b.class_vars),
-        tuple_intersection(a.metas, b.metas),
-    )
-
-    return intersection
+    def merge_domains(self, domains):
+        def fix_names(part):
+            for i, attr, name in zip(count(), part, name_iter):
+                if attr.name != name:
+                    part[i] = attr.renamed(name)
+                    self.Warning.renamed_variables()
+
+        oper = set.union if self.merge_type == OWConcatenate.MergeUnion \
+            else set.intersection
+        parts = [self._get_part(domains, oper, part)
+                 for part in ("attributes", "class_vars", "metas")]
+        all_names = [var.name for var in chain(*parts)]
+        name_iter = iter(get_unique_names_duplicates(all_names))
+        for part in parts:
+            fix_names(part)
+        domain = Orange.data.Domain(*parts)
+        return domain
+
+    @classmethod
+    def _get_part(cls, domains, oper, part):
+        # keep the order of variables: first compute union or intersections as
+        # sets, then iterate through chained parts
+        vars_by_domain = [getattr(domain, part) for domain in domains]
+        valid = reduce(oper, map(set, vars_by_domain))
+        valid_vars = [var for var in chain(*vars_by_domain) if var in valid]
+        return cls._unique_vars(valid_vars)
+
+    @staticmethod
+    def _unique_vars(seq: List[Orange.data.Variable]):
+        AttrDesc = namedtuple(
+            "AttrDesc",
+            ("template", "original", "values", "number_of_decimals"))
+
+        attrs = {}
+        for el in seq:
+            desc = attrs.get(el)
+            if desc is None:
+                attrs[el] = AttrDesc(el, True,
+                                     el.is_discrete and el.values,
+                                     el.is_continuous and el.number_of_decimals)
+                continue
+            if desc.template.is_discrete:
+                sattr_values = set(desc.values)
+                # don't use sets: keep the order
+                missing_values = [val for val in el.values
+                                  if val not in sattr_values]
+                if missing_values:
+                    attrs[el] = attrs[el]._replace(
+                        original=False,
+                        values=desc.values + missing_values)
+            elif desc.template.is_continuous:
+                if el.number_of_decimals > desc.number_of_decimals:
+                    attrs[el] = attrs[el]._replace(
+                        original=False,
+                        number_of_decimals=el.number_of_decimals)
+
+        new_attrs = []
+        for desc in attrs.values():
+            attr = desc.template
+            if desc.original:
+                new_attr = attr
+            elif desc.template.is_discrete:
+                new_attr = attr.copy()
+                for val in desc.values[len(attr.values):]:
+                    new_attr.add_value(val)
+            else:
+                assert desc.template.is_continuous
+                new_attr = attr.copy(number_of_decimals=desc.number_of_decimals)
+            new_attrs.append(new_attr)
+        return new_attrs
 
 
 if __name__ == "__main__":  # pragma: no cover