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] DataProjectionWidget: Update combos on new data #4405

Merged
merged 2 commits into from
Feb 10, 2020

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Feb 10, 2020

Issue

Fixes #4391

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@janezd
Copy link
Contributor

janezd commented Feb 10, 2020

At first I didn't understand why this fixes the problem. It is amazing that the widget worked with wrong variables in the model -- I guess it's only because we eliminated Variable.make, so the wrong variable matched the same-named variable from the new domain. :)

I added a test to convince myself that it's really about wrong variables in the model. And because it's prudent to have this test to prevent somebody from "re-optimizing" the code in the future.

@codecov
Copy link

codecov bot commented Feb 10, 2020

Codecov Report

Merging #4405 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4405   +/-   ##
=======================================
  Coverage   87.43%   87.44%           
=======================================
  Files         405      405           
  Lines       74061    74073   +12     
=======================================
+ Hits        64758    64772   +14     
+ Misses       9303     9301    -2     

@janezd janezd merged commit ecbbe9e into biolab:master Feb 10, 2020
@VesnaT
Copy link
Contributor Author

VesnaT commented Feb 11, 2020

Thanks for the test. I tried to write one, but it kept passing with the old code.

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.

Scatter plot does not detect coloring changes
2 participants