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: Scatter Plot automatically sends selection #2649

Merged
merged 1 commit into from
Oct 13, 2017
Merged

[FIX] Scatter Plot: Scatter Plot automatically sends selection #2649

merged 1 commit into from
Oct 13, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Oct 4, 2017

Issue

Fixes #2646

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Oct 4, 2017

Codecov Report

Merging #2649 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2649      +/-   ##
==========================================
- Coverage   75.52%   75.52%   -0.01%     
==========================================
  Files         332      332              
  Lines       58037    58046       +9     
==========================================
+ Hits        43832    43838       +6     
- Misses      14205    14208       +3

@@ -425,7 +425,8 @@ def update_graph(self, reset_view=True, **_):
self.graph.update_data(self.attr_x, self.attr_y, reset_view)

def selection_changed(self):
self.send_data()
if self.auto_send_selection:
self.send_data()
Copy link
Contributor

Choose a reason for hiding this comment

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

This half-reimplements the logic of Orange.widgets.gui.auto_commit. This does not work:

  • disable auto commit,
  • select some points,
  • enable auto commit.

Selected points are not sent.

The auto_commit function has a local variable dirty, which you would have to sent, but can't access from here.

A solution that would work would be to simply call self.commit instead of directly calling self.send_data. A (slight) problem with this is that it would re-send features when the selection is changed. This could be prevented by storing the last sent features and not sending them when not needed. Not most elegant solution, though.

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.

check_select(False)
self.assertIsNone(self.get_output(self.widget.Outputs.selected_data))
check_select(True)
self.assertIsInstance(self.get_output(self.widget.Outputs.selected_data), Table)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the test for the scenario I described above.

when the checkbox Send automatically is checked.
@janezd janezd merged commit 9b61bef into biolab:master Oct 13, 2017
@jerneju jerneju deleted the scatterplot-send-selection branch October 13, 2017 09:57
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.

3 participants