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

Adds option 'statistic' to get interquantile interval #975

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

adamryczkowski
Copy link

… instead of coniidence interval in error_statistic function that gets called by the ggplot.resamples.

It is my solution to the issue #952

It can be used as e.g.

res<-caret::resamples(mymodel)
ggplot(res,metric="RMSE",main="GBM vs xgboost", statistic = 'IR')

…idence interval in error_statistic function that gets called by the ggplot.resamples
@codecov-io
Copy link

codecov-io commented Nov 26, 2018

Codecov Report

Merging #975 into master will decrease coverage by 13.12%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #975       +/-   ##
==========================================
- Coverage   32.63%   19.5%   -13.13%     
==========================================
  Files          90      90               
  Lines       13017   12114      -903     
==========================================
- Hits         4248    2363     -1885     
- Misses       8769    9751      +982
Impacted Files Coverage Δ
R/getTrainPerf.R 0% <0%> (-100%) ⬇️
R/diag.R 0% <0%> (-100%) ⬇️
R/updates.R 0% <0%> (-61.77%) ⬇️
R/safs.R 5.74% <0%> (-56.58%) ⬇️
R/gafs.R 8.83% <0%> (-55.97%) ⬇️
R/dummyVar.R 0% <0%> (-48.6%) ⬇️
R/selectByFilter.R 0% <0%> (-46.96%) ⬇️
R/rfe.R 0% <0%> (-45.85%) ⬇️
R/resamples.R 0% <0%> (-17.65%) ⬇️
R/reg_sims.R 0% <0%> (-14.29%) ⬇️
... and 52 more

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 6e3d92d...ca58a5a. Read the comment docs.

@topepo
Copy link
Owner

topepo commented Mar 24, 2019

Can you please add documentation for the parameter, add a unit test, run R CMD check, and update the NEWS file too?

@adamryczkowski
Copy link
Author

OK. I will look at that in my spare time.

@topepo
Copy link
Owner

topepo commented Sep 20, 2019

Getting ready to merge this. Can you write/update units tests related to this (and update the NEWS file)? The PR fails travis

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