-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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] Fixes for deprecations in 3.26, and for changed behaviour of file dialog #4643
Changes from all commits
ca11cc1
d4c1666
4a0cd77
d89e896
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -248,9 +248,9 @@ class Outputs: | |
|
||
correlation_type: int | ||
|
||
settings_version = 2 | ||
settings_version = 3 | ||
settingsHandler = DomainContextHandler() | ||
selection = ContextSetting(()) | ||
selection = ContextSetting([]) | ||
feature = ContextSetting(None) | ||
correlation_type = Setting(0) | ||
|
||
|
@@ -307,7 +307,7 @@ def _feature_combo_changed(self): | |
self.apply() | ||
|
||
def _vizrank_selection_changed(self, *args): | ||
self.selection = [(var.name, vartype(var)) for var in args] | ||
self.selection = list(args) | ||
self.commit() | ||
|
||
def _vizrank_stopped(self): | ||
|
@@ -323,7 +323,7 @@ def _vizrank_select(self): | |
# filtered by a feature and therefore selection could not be found | ||
selection_in_model = False | ||
if self.selection: | ||
sel_names = sorted(name for name, _ in self.selection) | ||
sel_names = sorted(var.name for var in self.selection) | ||
for i in range(model.rowCount()): | ||
# pylint: disable=protected-access | ||
names = sorted(x.name for x in model.data( | ||
|
@@ -345,7 +345,7 @@ def set_data(self, data): | |
self.clear_messages() | ||
self.data = data | ||
self.cont_data = None | ||
self.selection = () | ||
self.selection = [] | ||
if data is not None: | ||
if len(data) < 2: | ||
self.Warning.not_enough_inst() | ||
|
@@ -415,7 +415,7 @@ def commit(self): | |
|
||
# data has been imputed; send original attributes | ||
self.Outputs.features.send(AttributeList( | ||
[self.data.domain[name] for name, _ in self.selection])) | ||
[self.data.domain[var.name] for var in self.selection])) | ||
self.Outputs.correlations.send(corr_table) | ||
|
||
def send_report(self): | ||
|
@@ -426,8 +426,12 @@ def send_report(self): | |
def migrate_context(cls, context, version): | ||
if version < 2: | ||
sel = context.values["selection"] | ||
context.values["selection"] = ([(var.name, vartype(var)) | ||
for var in sel[0]], sel[1]) | ||
context.values["selection"] = [(var.name, vartype(var)) | ||
for var in sel[0]] | ||
if version < 3: | ||
sel = context.values["selection"] | ||
context.values["selection"] = ([(name, vtype + 100) | ||
for name, vtype in sel], -3) | ||
Comment on lines
+429
to
+434
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused, isn't selection now supposed to be a list of variables? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The widget's attribute (as in In settings version 2 the widget had an attribute In settings version 3, the widget's Coincidentally, the encoded setting looks (almost) the same, because the widget's encoding was (almost) the same as encoding by the handler. So migration just adds |
||
|
||
|
||
if __name__ == "__main__": # pragma: no cover | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.data.domain[var.name]
is a bit redundant, domain can be indexed with variables as well as names:self.data.domain[var]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work even if var is loaded from settings and is not the same object right? Should have the same hash even in those exotic cases with compute_values etc? If not sure you can always ignore my comment above and leave it as it is now :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so, too, but it doesn't work.
compute_value
is a part of a hash (and has to be, otherwise domain transformation would mistook two variables to be equivalent when they are not).We have to use
var.name
here.