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] Impute: Fix state serialization/restore #2830

Merged

Conversation

ales-erjavec
Copy link
Contributor

Issue
File ---------------> Impute
                     ^
Random forest ------/

Change some individual variable imputer to Model based (Random forest)

Remove the Impute widget. Create a new Impute widget and connect it to File again (without connecting it to any learner):

File ---> Impute

with the same file opened in File. The Impute widget restores the variables' state as model (Random forest) but there is no Random forest input.

The problem is that Impute stores actual constructed Imputer instances in its settings.

Description of changes

Explicitly model what the widget does and not only its (by) products.

Includes
  • Code changes
  • Tests
  • Documentation

return type(method), None

methods = set(method_key(m) for m in methods)
variables = [self.varmodel[i.row()] for i in indexes]
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to be unused,
and is the same as selected_vars below.

@ales-erjavec ales-erjavec force-pushed the fixes/impute-proper-state-again branch from 7117ef3 to 24a8df2 Compare December 28, 2017 11:37
@codecov-io
Copy link

codecov-io commented Dec 28, 2017

Codecov Report

Merging #2830 into master will increase coverage by 0.03%.
The diff coverage is 90.62%.

@@            Coverage Diff             @@
##           master    #2830      +/-   ##
==========================================
+ Coverage   82.12%   82.15%   +0.03%     
==========================================
  Files         329      329              
  Lines       56258    56283      +25     
==========================================
+ Hits        46201    46239      +38     
+ Misses      10057    10044      -13

Model what the widget does not what it creates
@ales-erjavec ales-erjavec force-pushed the fixes/impute-proper-state-again branch from 24a8df2 to b277ebb Compare December 30, 2017 13:11
@lanzagar lanzagar added this to the 3.10 milestone Jan 19, 2018
@ales-erjavec ales-erjavec force-pushed the fixes/impute-proper-state-again branch from b277ebb to f8591bd Compare January 29, 2018 10:06
@lanzagar lanzagar merged commit 23f3ca9 into biolab:master Feb 2, 2018
@ales-erjavec ales-erjavec deleted the fixes/impute-proper-state-again branch February 13, 2018 16:13
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