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] owcorrespondence: Handle variables with one value #2066

Merged
merged 1 commit into from
Mar 10, 2017
Merged

[FIX] owcorrespondence: Handle variables with one value #2066

merged 1 commit into from
Mar 10, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Mar 2, 2017

Issues
  1. IndexError: index 1 is out of bounds for axis 1 with size 1
  2. SVD did not converge
  3. Must specify at least one of rect, xRange, or yRange. (gave rect=<class 'NoneType'>)

Check that the widget does not crash when domain has a discrete variable with only one value.

Description of changes

Discrete variables with only one value are not taken into account.

Includes
  • Code changes
  • Tests
  • Documentation

@jerneju
Copy link
Contributor Author

jerneju commented Mar 2, 2017

@codecov-io
Copy link

codecov-io commented Mar 2, 2017

Codecov Report

Merging #2066 into master will increase coverage by 0.2%.
The diff coverage is 75%.

@@            Coverage Diff            @@
##           master    #2066     +/-   ##
=========================================
+ Coverage   69.93%   70.14%   +0.2%     
=========================================
  Files         315      315             
  Lines       53979    54001     +22     
=========================================
+ Hits        37752    37878    +126     
+ Misses      16227    16123    -104

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1165800...98e2839. Read the comment docs.

@jerneju jerneju changed the title [FIX] IndexError - owcorresondence [FIX] IndexError - owcorrespondence Mar 2, 2017
@@ -108,7 +107,7 @@ def set_data(self, data):
self.data = data
if data is not None:
self.varlist[:] = [var for var in data.domain.variables
if var.is_discrete]
if var.is_discrete and len(var.values) > 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

This only filters out variables with less than 2 values.
A variable can have 3 values, but after selecting a subset only instances with 1 will remain.
This widget will still crash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I get "SVD did not converge" error.

@lanzagar
Copy link
Contributor

lanzagar commented Mar 3, 2017

When a PR is ready for review and merging, please try to give it a good name (that can appear in a Changelog) - the python error from the bug is not the most informative.
E.g. this could be:
[FIX] owcorrespondence: Handle variables with one value

@lanzagar lanzagar self-assigned this Mar 3, 2017
@jerneju jerneju changed the title [FIX] IndexError - owcorrespondence [FIX] owcorrespondence: Handle variables with one value Mar 6, 2017
@jerneju
Copy link
Contributor Author

jerneju commented Mar 6, 2017

@jerneju
Copy link
Contributor Author

jerneju commented Mar 6, 2017

Errors thrown:
1. IndexError: index 1 is out of bounds for axis 1 with size 1
2. SVD did not converge
3. Must specify at least one of rect, xRange, or yRange. (gave rect=<class 'NoneType'>)
Problems caused by:
1) Domain has a two or more discrete variables but less than in a table
2) There is at least one NaN value in a column.
@astaric astaric merged commit aa30c1f into biolab:master Mar 10, 2017
@jerneju jerneju deleted the indexerror-owcorrespondence branch April 20, 2017 13:47
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.

4 participants