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] Linear Projection: Fix LDA #5828

Merged
merged 2 commits into from
Feb 11, 2022
Merged

[FIX] Linear Projection: Fix LDA #5828

merged 2 commits into from
Feb 11, 2022

Conversation

lanzagar
Copy link
Contributor

@lanzagar lanzagar commented Feb 4, 2022

Issue

Fixes #5359

Description of changes

I thought about removing the overriden effective_data (which also fixes LDA), but then saw that it was done based on a talk between @janezd and @VesnaT (#4846 (comment)) who must have had a good reason for it.
So I just added the class to effective_data, but @janezd or @VesnaT should confirm that that makes sense.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Feb 4, 2022

Codecov Report

Merging #5828 (1bf5e7e) into master (3516fc3) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5828      +/-   ##
==========================================
+ Coverage   86.20%   86.21%   +0.01%     
==========================================
  Files         316      316              
  Lines       66618    66630      +12     
==========================================
+ Hits        57426    57444      +18     
+ Misses       9192     9186       -6     

@VesnaT
Copy link
Contributor

VesnaT commented Feb 7, 2022

Adding the class to effective_data makes sense, but the _invalidated/_domain_invalidated flags should be changed accordingly.

The following example does not work correctly:
1 . File (Iris) -> Select Columns (Reset) -> Linear Projection (LDA)
2. In the Select Columns move Iris to Ignored -> In Linear Projection the Circular Placement is selected and LDA is disable but the plot is not changed.
3. In the Select Columns move Iris back to Target -> In Linear Projection the Circular Placement in selected and LDA enabled but the plot is still not changed.

@@ -329,7 +329,8 @@ def effective_variables(self):

@property
def effective_data(self):
return self.data.transform(Domain(self.effective_variables))
return self.data.transform(Domain(self.effective_variables,
self.data.domain.class_vars))
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realised this wont work because class variable is duplicated when selected in effective variables.

@VesnaT VesnaT merged commit b0fbaa1 into biolab:master Feb 11, 2022
@lanzagar lanzagar deleted the fix_lda branch March 14, 2022 14:39
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.

LDA not working in Linear Projection widget ?
2 participants