Skip to content

Conversation

@joaquinvanschoren
Copy link
Contributor

Reference Issue

What does this PR implement/fix? Explain your changes.

How should this PR be tested?

Any other comments?

@mfeurer
Copy link
Collaborator

mfeurer commented Apr 25, 2018

Thanks, do you maybe want to update the example, too? Also, could you please redirect your PR to the development branch?

@joaquinvanschoren joaquinvanschoren changed the base branch from master to develop April 25, 2018 11:35
@joaquinvanschoren
Copy link
Contributor Author

joaquinvanschoren commented Apr 25, 2018

On it.
Incidentally, don't we want to have a shorthand for sklearn, i.e. instead of doing

flow = flows.sklearn_to_flow(clf)
run = runs.run_flow_on_task(task, flow)

do something like

run = runs.run_sklearn_on_task(clf, task)

From what I've seen, people always use those two lines together without doing anything with the flow object.

@mfeurer
Copy link
Collaborator

mfeurer commented Apr 25, 2018

We have run_model_on_task.

@joaquinvanschoren
Copy link
Contributor Author

I updated the example, but there was nothing really wrong with it?

Copy link
Collaborator

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

Thanks, actually I meant the IPython notebook. It contains installation instructions, too.

doc/index.rst Outdated
run.publish()
print('URL for run: %s/run/%d' % (openml.config.server, run.run_id))
print('View the run online: https://www.openml.org/r/%d' % run.run_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this one is redundant given the line above.

@mfeurer
Copy link
Collaborator

mfeurer commented Jun 14, 2018

@joaquinvanschoren I did some more changes - could you please also have a look again and approve it if you like it?

@joaquinvanschoren
Copy link
Contributor Author

Hmm, Travis fails because it can't find numpy in the Python 3.4 test. What should I do about this?

@codecov-io
Copy link

Codecov Report

Merging #456 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #456   +/-   ##
========================================
  Coverage    89.46%   89.46%           
========================================
  Files           32       32           
  Lines         2771     2771           
========================================
  Hits          2479     2479           
  Misses         292      292

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 4603345...d24c04c. Read the comment docs.

@joaquinvanschoren joaquinvanschoren merged commit 351e2b7 into develop Jun 18, 2018
@joaquinvanschoren
Copy link
Contributor Author

Oh oh, it seemed like Travis finished successfully, but then suddenly there was still a fail after I merged it.
The issue is that Travis fails because it can't find numpy in the Python 3.4 test. Any ideas?

@mfeurer mfeurer deleted the joaquinvanschoren-patch-1 branch September 13, 2018 07:18
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.

4 participants