[ENH] Visualize widgets: Output Annotated data and Fixups #1677
Conversation
| @@ -1146,7 +1146,7 @@ def commit(self): | |||
| selected_data = data[mask] | |||
| if self.append_clusters: | |||
| def remove_other_value(vars_): | |||
| vars_ = [var for var in vars_] | |||
| vars_ = list(vars_).copy() | |||
There was a problem hiding this comment.
Ooops, in my earlier comment I thought that vars_ was a list. Sorry. Now that you have to convert it to a list anyway, list(vars_) will suffice; you can skip copy().
VesnaT
force-pushed
the
annotated_data
branch
from
d9eec0f
to
987e0ca
Compare
Current coverage is 89.39% (diff: 100%)@@ master #1677 diff @@
==========================================
Files 79 79
Lines 8603 8603
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 7691 7691
Misses 912 912
Partials 0 0
|
| all((var in [var.name for var in annotated.domain.metas] | ||
| for var in [var.name for var in selected.domain.metas]))) | ||
| self.assertLess(set(var.name for var in selected.domain.metas), | ||
| set(var.name for var in annotated.domain.metas)) |
There was a problem hiding this comment.
No need to change anything here, but: you can also use set comprehensions in such cases. (I keep forgetting this, too.)
| @@ -224,6 +229,8 @@ def set_tree(self, model=None): | |||
| # if hasattr(model, 'meta_depth_limit'): | |||
| # self.depth_limit = model.meta_depth_limit | |||
| # self.update_depth() | |||
| self.send(ANNOTATED_DATA_SIGNAL_NAME, | |||
There was a problem hiding this comment.
Is this needed? The "Selected data" signal is not sent here. I commented it out and it seems it still works.
| @@ -347,6 +355,11 @@ def commit(self): | |||
| data = self.tree_adapter.get_instances_in_nodes( | |||
| self.clf_dataset, [item.tree_node.label for item in items]) | |||
| self.send('Selected Data', data) | |||
| nodes = [i.tree_node.label for i in self.scene.selectedItems() | |||
There was a problem hiding this comment.
Isn't this already computed above, as the second argument in the call of get_instances_in_nodes?
| @@ -18,8 +20,9 @@ class OWRuleViewer(widget.OWWidget): | |||
| inputs = [("Data", Table, 'set_data'), | |||
| ("Classifier", _RuleClassifier, 'set_classifier')] | |||
|
|
|||
| data_output_identifier = "Filtered data" | |||
| outputs = [(data_output_identifier, Table)] | |||
| data_output_identifier = "Selected Data" | |||
There was a problem hiding this comment.
I hate this.
First, it should be upper case -- if for nothing else, to match the case of ANNOTATED_DATA_SIGNAL_NAME.
Second, it's probably the only widget which defines a constant for the signal name. This could be a good idea, but only if we do it elsewhere too.
It's not your fault (it was here before), and removing it is not a part of this PR. But since you have to rename it anyway, you could perhaps simply get rid of it. Just a suggestion, you don't have to.
VesnaT
force-pushed
the
annotated_data
branch
from
987e0ca
to
2f7246a
Compare
|
@VesnaT, a small rebase, please. |
VesnaT
force-pushed
the
annotated_data
branch
2 times, most recently
from
cc350f6
to
86e2ebd
Compare
VesnaT
force-pushed
the
annotated_data
branch
from
86e2ebd
to
635ba39
Compare
No description provided.