Amendments and additional test for PR #624 (issue #620)

[m-dz]

Amendments and additional test for PR #624 (issue #620).

format() is added to facets' labels (which I am not sure is needed, as it is more a matter of taste and consistency), without it test labels were as follows:

1. "Subsample Ratio of Columns: 1e-04";
2. "Subsample Ratio of Columns: 0.01";
3. "Subsample Ratio of Columns: 1";

which is now changed to:

1. "Subsample Ratio of Columns: 1e-04";
2. "Subsample Ratio of Columns: 1e-02";
3. "Subsample Ratio of Columns: 1e+00".

Also additional test for ordering of facets' labels is added and packages kernlab and xgboost are explicitly loaded as the "missing package" error message when using library() is much clearer compared to the whole call stack print without it:

Error message with library():

Failed -------------------------------------------------------------------------
1. Error: ggplot.train correctly orders factors (@test_ggplot.R#5) -------------
there is no package called 'kernlab'
1: library(kernlab) at [removed]\caret\pkg\caret/tests/testthat/test_ggplot.R:5
2: stop(txt, domain = NA)

and without:

Test ggplot: 1 package is needed for this model and is not installed. (kernlab). Would you like to try to install it now?1..

Failed -------------------------------------------------------------------------
1. Error: ggplot.train correctly orders factors (@test_ggplot.R#7) -------------

1: train(mpg ~ cyl + disp, data = mtcars, method = "svmRadial", tuneGrid = expand.grid(C = 1:2, 
       sigma = c(1e-04, 0.01, 1))) at [removed]\caret\pkg\caret/tests/testthat/test_ggplot.R:7
2: train.formula(mpg ~ cyl + disp, data = mtcars, method = "svmRadial", tuneGrid = expand.grid(C = 1:2, 
       sigma = c(1e-04, 0.01, 1))) at [removed]\caret\pkg\caret/R/train.default.R:217
3: train(x, y, weights = w, ...) at [removed]\caret\pkg\caret/R/train.default.R:912
4: train.default(x, y, weights = w, ...) at [removed]\caret\pkg\caret/R/train.default.R:217
5: checkInstall(models$library) at [removed]\caret\pkg\caret/R/train.default.R:251
6: stop() at [removed]\caret\pkg\caret/R/modelLookup.R:110

[m-dz]

I can see that tests are failing on loading xgboost ("there is no package called 'xgboost'" in the Travis log), should I remove this test or the missing package can somehow be added?

[topepo]

I added xgboost to the travis installer code.

I'd like to avoid adding xgboost to the DESCRIPTION field so can you add an "skip on cran" flag on that particular test and then double check that R CMD check --as-cran doesn't complain?

[topepo]

Can you sync your repo? That will update the travis file on your end.

[m-dz]

I will do my best!

[m-dz]

After adding skip_on_cran() I was able to build the package and "almost successfully" check it with --as-cran with 3 distinct warnings:

• Timed out connection to http://www.jstatsoft.org/article/view/v028i05/v28i05.pdf (link also not working on my web browser, have to use https one: https://www.jstatsoft.org/article/view/v028i05/v28i05.pdf);
• Warning: package 'lattice' was built under R version 3.3.3
• 'qpdf' is needed for checks on size reduction of PDFs

and a "pdflatex is not available" error when building vignettes.

I would risk to say those can be ignored, but I would try to fix "pdflatex" anyway (I am not sure where to start as pdflatex --version is working correctly returning version 2.9.6211).

Edit: And apologies for the "Merge remote-tracking branch 'upstream/master' into fix_620-patch" commit, had to merge your master to my fix_620-patch. Now I think I could have amended the whole merging commit or something...

[coveralls]

Coverage decreased (-2.5%) to 16.997% when pulling 3008a10 on m-dz:fix_620-patch into bb1d58b on topepo:master.

[m-dz]

Unfortunately it is xgboost again in NOT_CRAN="true" job, I will get back to it tomorrow evening (UTC).

[topepo]

One easy fix would be to use another model that is not in xgboost as a test case, preferably one that is already in a package loaded by .travis.yml :-/

[m-dz]

Yes, that is one way to go I thought about, but to test this we need to have a model with at least 3 parameters, preferably in the 0-1 interval, and if I remember right I could not find the right one in the packages already in travis. I will have a second look!

[topepo]

Maybe RDA or polynomial SVMs?

[coveralls]

Coverage increased (+0.07%) to 19.573% when pulling 142abb3 on m-dz:fix_620-patch into e5ae884 on topepo:master.

[coveralls]

Coverage increased (+0.07%) to 19.573% when pulling 142abb3 on m-dz:fix_620-patch into e5ae884 on topepo:master.

[m-dz]

Looks like everything is working, if you are happy with it I will clean the branch tomorrow (i.e. remove this PR and do another one with a single commit).

[topepo]

Thanks

[topepo]

Ooops. Sorry to accidentally close it. I'd like to merge soon to get it into the CRAN submission tomorrow.

[coveralls]

Coverage increased (+0.07%) to 19.573% when pulling 142abb3 on m-dz:fix_620-patch into e5ae884 on topepo:master.

[m-dz]

Sure, I will prepare the new pull request tomorrow morning at the latest. Are you happy with all changes?

[topepo]

Yes. They looked fine to me. On April 12, 2017 at 3:18:57 PM, m-dz (notifications@github.com) wrote: Sure, I will prepare the new pull request tomorrow morning at the latest. Are you happy with all changes? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#630 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFdy4y0HRU04MvU79HNW4Kt3LTfMRAk3ks5rvSOhgaJpZM4M08l8> .

[coveralls]

Coverage increased (+0.07%) to 19.573% when pulling 7967d20 on m-dz:fix_620-patch into aae4ee4 on topepo:master.

[m-dz]

Should be ready to merge.

[topepo]

Thanks

[m-dz]

It was a pleasure and a great exercise, thanks as well.