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] Color widget failed after removing Variable.make #4041

Merged
merged 3 commits into from
Oct 8, 2019

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Sep 20, 2019

Issue

After removing Variable.make, OWColor fails when loading settings because it tries to set Variable.name when loading settings.

The widget was misbehaving badly. set_data constructed the output table(!) and modified its attributes (not only colors but also names!). This most probably also broke Domain's dictionary _indices.

Description of changes

Fixing this required reimplementing the way in which the widget stores the data. Instead of storing variables, it stores the data (names, colors) and constructs variables before sending output.

#3822 could have also been the result of the questionable behaviour of the widget and may be fixed with this PR.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Sep 20, 2019

Codecov Report

Merging #4041 into master will increase coverage by 0.07%.
The diff coverage is 96.64%.

@@            Coverage Diff             @@
##           master    #4041      +/-   ##
==========================================
+ Coverage   85.47%   85.55%   +0.07%     
==========================================
  Files         385      385              
  Lines       69107    69059      -48     
==========================================
+ Hits        59068    59080      +12     
+ Misses      10039     9979      -60

@janezd janezd force-pushed the owcolor-fix-after-unmake branch 2 times, most recently from d006834 to e9d639b Compare September 22, 2019 08:41
@markotoplak markotoplak self-assigned this Oct 4, 2019
@markotoplak
Copy link
Member

@janezd: you intentionally deleted ordered_values, right?

@janezd
Copy link
Contributor Author

janezd commented Oct 8, 2019

@janezd: you intentionally deleted ordered_values, right?

Yes. We haven't use them, as far as I know and check.

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