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] Scatter Plot: two minor errors #2381

Merged
merged 2 commits into from
Jun 23, 2017
Merged

[FIX] Scatter Plot: two minor errors #2381

merged 2 commits into from
Jun 23, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Jun 6, 2017

Issue

screenshot_20170606_120406

Steps to reproduce the behavior

Put Scatter Plot widget on the canvas. Then try to change shape. Select (Same shape).

screenshot_20170606_120429

Steps to reproduce the behavior

Put Scatter Plot widget on the canvas. Then do something. For instance, try to change color. Find Informative Projections button enables and it crashes when an user starts vizrank.

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@jerneju
Copy link
Contributor Author

jerneju commented Jun 6, 2017

@jerneju
Copy link
Contributor Author

jerneju commented Jun 6, 2017

@codecov-io
Copy link

codecov-io commented Jun 6, 2017

Codecov Report

Merging #2381 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2381      +/-   ##
==========================================
- Coverage   74.25%   74.22%   -0.04%     
==========================================
  Files         320      320              
  Lines       55786    55785       -1     
==========================================
- Hits        41425    41405      -20     
- Misses      14361    14380      +19

@jerneju jerneju changed the title [FIX] Scatter Plot: two minor errors [FIX] Scatter Plot: three minor errors Jun 8, 2017
@jerneju jerneju changed the title [FIX] Scatter Plot: three minor errors [FIX] Scatter Plot: two minor errors Jun 14, 2017
@lanzagar lanzagar self-assigned this Jun 16, 2017
@@ -989,7 +989,7 @@ def update_shapes(self):
self.make_legend()

def assure_attribute_present(self, attr):
if attr not in self.data.domain:
if self.data and attr not in self.data.domain:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use self.data is not None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

self.vizrank_button.setEnabled(
self.data is not None and self.data.domain.class_var is not None
and len(self.data.domain.attributes) > 1 and len(self.data) > 1)
self._vizrank_button_enabled()
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that the bug that you are fixing here is that the button was not enabled when it should have been. This shouldn't be "fixed" so that the button is always enabled. These conditions were correct, so the bug -- the reason that this line was not executed -- must be somewhere else.

You should find the actual error and fix it instead of removing the code that (I suppose) works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I moved code somewhere else. Now I have put it back here and added check for sparse.

@@ -423,7 +421,7 @@ def prepare_data(self):
new=False)

def sparse_to_dense(self, input_data=None):
self.vizrank_button.setEnabled(not (self.data and self.data.is_sparse()))
self._vizrank_button_enabled()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@janezd
Copy link
Contributor

janezd commented Jun 21, 2017

What should I do to reproduce the error? Please provide actual descriptions; these screenshots are not informative.

@jerneju
Copy link
Contributor Author

jerneju commented Jun 23, 2017

@janezd : Instructions to reproduce these two errors are now added above.

@jerneju
Copy link
Contributor Author

jerneju commented Jun 23, 2017

@janezd janezd merged commit 7636e49 into biolab:master Jun 23, 2017
@jerneju jerneju deleted the two-errors-scatterplot branch June 23, 2017 14:35
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