Skip to content

[FIX] Projections: Retain embedding if non-relevant variables change #3428

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

Merged
merged 3 commits into from
Nov 30, 2018

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Nov 30, 2018

Issue

Embedding was recalculated and the graph reploted even though the variables used remain the same with new input data.

Description of changes

Adjust OWDataProjectionWidget not to recalculate the embedding.
Consider MDS separately since it takes two inputs (DataTable, DistMatrix).
Fix OWRadViz and OWLinearProjection not to plot data until available features list box is set.

Includes
  • Code changes
  • Tests
  • Documentation

@janezd janezd self-assigned this Nov 30, 2018
@VesnaT VesnaT force-pushed the optimize_projections branch from 8c14d66 to ced1ca9 Compare November 30, 2018 09:20
@codecov
Copy link

codecov bot commented Nov 30, 2018

Codecov Report

Merging #3428 into master will decrease coverage by <.01%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master    #3428      +/-   ##
==========================================
- Coverage   83.17%   83.17%   -0.01%     
==========================================
  Files         362      362              
  Lines       63832    63857      +25     
==========================================
+ Hits        53094    53110      +16     
- Misses      10738    10747       +9

@VesnaT VesnaT force-pushed the optimize_projections branch from ced1ca9 to 34e0a2f Compare November 30, 2018 10:40
# Input
@Inputs.data
@check_sql_input
def set_data(self, data):
same_domain = (self.data and data and
data_exists = self.data is not None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data_existed

@@ -552,8 +573,6 @@ def sizeHint(self):
return QSize(1132, 708)

def clear(self):
self.data = None
self.valid_data = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do this in inherited check_data.

self.graph.set_effective_matrix(None)
self.__set_update_loop(None)
self.__state = OWMDS.Waiting

def _initialize(self):
matrix_exists = self.effective_matrix is not None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existed


def clear(self):
def clear(self, with_attr_values=True):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove flag, call init_attr_values instead

@VesnaT VesnaT force-pushed the optimize_projections branch from 34e0a2f to b6c23c2 Compare November 30, 2018 11:31
@VesnaT VesnaT force-pushed the optimize_projections branch from b6c23c2 to 1a519d7 Compare November 30, 2018 12:26
@janezd
Copy link
Contributor

janezd commented Nov 30, 2018

I'll merge this today, but I'll wait to merge #2556 first, so that in case of conflicts we will rebase this PR and not #2556 again.

@janezd janezd merged commit 9c539ab into biolab:master Nov 30, 2018
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