Skip to content

Issue210#223

Merged
mfeurer merged 11 commits intodevelopfrom
issue210
Apr 20, 2017
Merged

Issue210#223
mfeurer merged 11 commits intodevelopfrom
issue210

Conversation

@janvanrijn
Copy link
Copy Markdown
Member

  • Fixed issue 210 by making a copy of the model for every CV fold (this is important to prevent data leakage).
  • Restructured the way an arff trace is generated (arff content and arff header are extracted separately, this was needed because of new assumptions and in order to keep the code clean after previous point)
  • Apparently, I also added the 'upload incompatible run with error message' in this branch. Sorry, but it's it's just two additional lines anyway.

@mfeurer
Copy link
Copy Markdown
Collaborator

mfeurer commented Apr 11, 2017

Unittests are broken.

disallowed the run of 'illegal' combinations (e.g., regression on classification)
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 20, 2017

Codecov Report

Merging #223 into develop will decrease coverage by 0.42%.
The diff coverage is 95.65%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #223      +/-   ##
===========================================
- Coverage    89.15%   88.72%   -0.43%     
===========================================
  Files           23       23              
  Lines         1807     1818      +11     
===========================================
+ Hits          1611     1613       +2     
- Misses         196      205       +9
Impacted Files Coverage Δ
openml/runs/run.py 91.24% <85.71%> (-0.71%) ⬇️
openml/runs/functions.py 83.8% <97.43%> (-2.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4df535...c23b113. Read the comment docs.

Comment thread openml/flows/sklearn_converter.py Outdated
return True
count = 0
returnValue = None
if isinstance(model, sklearn.pipeline.Pipeline):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I do not see any usecase for this. Do you have one in mind?

Comment thread openml/runs/functions.py
model_classes = model.classes_
if get_traceble_model(model_fold):
arff_tracecontent.extend(_extract_arfftrace(model_fold, rep_no, fold_no))
model_classes = model_fold.best_estimator_.classes_
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the worst case, the model classes can be different for each fold. Do we cope for that?

@mfeurer mfeurer merged commit 4b15e84 into develop Apr 20, 2017
@mfeurer mfeurer deleted the issue210 branch April 20, 2017 15:29
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