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: Always setup plot #3450

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Dec 7, 2018

Issue

When inputing features and data into Scatterplot at the same time handleNewSignals() is invoked only once (handleNewSignals() then calls setup_plot()). Therefore attr_x and attr_y, set from settings, could be the same as input features and self.setup_plot() would never be called.

Description of changes

Always call self.attr_changed() when receiving Features on the input.

Includes
  • Code changes
  • Tests
  • Documentation

@janezd
Copy link
Contributor

janezd commented Dec 7, 2018

Why do we need to call setup_plot when feature input does not actually change the current attr_x and attr_y? Does it have any useful side effects or does it just trigger an unnecessary update?

@janezd
Copy link
Contributor

janezd commented Dec 7, 2018

Answer to self after getting an answer from @VesnaT: if this input features signal is present, super().handleNewSignals() is not called because it's in the else part. The fix is rather dangerous though, because super().handleNewSignals() is still not called; it works just because attr_changed() calls all the same methods that are called by handleNewSignals(), but this can change in the future.

We can merge this PR out of urgency, but this has to be fixed by setting __invalidate to True instead.

@janezd janezd merged commit 7d48fe6 into biolab:master Dec 7, 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