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

initial hyperparameter listing #378

Merged
merged 17 commits into from
Oct 13, 2017
Merged

Conversation

giuseppec
Copy link
Member

Adresses #348 still needs documentation and tests.
Is a first working (and hacky) version, which needs to be improved!

@jakob-r , @DanielKuehn87 could you check if your usecases work and make unit tests out of your use cases?
I think we are not uploading default hyperparameters from mlr learners to OpenML and don't know which effect this will have here.

@codecov
Copy link

codecov bot commented Jul 10, 2017

Codecov Report

Merging #378 into master will decrease coverage by 0.07%.
The diff coverage is 92.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   96.35%   96.28%   -0.08%     
==========================================
  Files          58       59       +1     
  Lines        2305     2342      +37     
==========================================
+ Hits         2221     2255      +34     
- Misses         84       87       +3
Impacted Files Coverage Δ
R/helpers.R 97.36% <100%> (+0.04%) ⬆️
R/download.R 88.09% <100%> (ø) ⬆️
R/listOMLSetup.R 91.42% <91.42%> (ø)

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 42b2637...8a5aed3. Read the comment docs.

@jakob-r
Copy link
Collaborator

jakob-r commented Jul 11, 2017

Currently all columns are strings including flow.id and setup.id. I wanted to fix it myself but I could not really see how it is done in the other functions. There also seems to be some unused import for type.convert.

@giuseppec
Copy link
Member Author

Hm... I think we simply forgot to use type.convert after we moved from XML to JSON. We use as.integer for columns where we know that they will be integers.
But I would agree to use type.convert in general, I did this a few days ago anyway https://github.com/openml/openml-r/blob/master/R/listOMLStudies.R#L22 . Will open up a separate issue for this.

@giuseppec giuseppec merged commit cb2f557 into master Oct 13, 2017
@giuseppec giuseppec deleted the fix_348_list_hyperparameters branch November 2, 2017 09:47
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