[R-package] increase the code coverage of R-package about rgf HOT 16 CLOSED

Hey yeah I was thinking about that! We should make codecov fail builds as a sign that the R tests got skipped.

Yeah CRAN (very reasonably) does not let you know install stuff on their machines. This is something my friends @bburns632 and @jayqi and I working on pkgnet had to do some WILD stuff to get around (pkgnet#108 if you're curious).

I think to feel confident in the R tests here we'd need to run them on Travis AND explicitly put in a check on codecov that says "if coverage falls below threshold, fail build". @mlampros if you agree I can create an issue for this and try to address it.

@StrikerRUS to your comment "I'm completely unfamiliar with R", I can explain the root of the issue here. Basically most machine learning libraries that follow the pattern of "implement in something low-level then write APIs in Python and R" have native code in R or Python calling the low-level code. So e.g. for LightGBM R-package we're using Rcpp and the built-ins like .Call() to directly run C++ code.

RGF is different in that it does that in Python and then uses an R package called reticulate to call the Python code. So instead of R --> C++ we're going R --> Python --> C++.

This is bad in the sense that you need to have a full Python setup to run the R stuff, but it's good in the sense that development can focus almost exclusively on the Python side and making the R package work only means ensuring it plays nicely with Python.

from rgf.

@jameslamb Hi!

Just letting you know that the code coverage of R-package is now checked at the CI side with a 50%-threshold.

from rgf.

After #311 and #312 have been merged, it seems that all code lines related to sparse features are covered:

@mlampros May I kindly ask you to help with writing tests for the rest features?

from rgf.

yes definitely! I host an open source hack night at my company every two weeks. Next one is next Wednesday...mind if I unleash the attendees on this?

Seems like a good learning opportunity and a good task for people who know R but are learning how open source works. And also it would be valuable for the project!

from rgf.

@StrikerRUS I'm ok with @jameslamb 's idea.

from rgf.

Wow, that's cool! Many thanks!

from rgf.

Great, thanks!

from rgf.

@jameslamb Hi!

Can't wait to see your R-ninjas here! 😄

from rgf.

@StrikerRUS hey my team and I did spend a few hours working on hits. Unfortunately we were never able to get past the "skip if Python modules don't exist" checks (see my comment on #276 ).

I am definitely interested in coming back and trying but can say it was pretty difficult to get the tests to run :/

from rgf.

It's something I'm curious, maybe you can tell me...did CRAN ever complain about the use of reticulate for the R package? That's not a pattern I've seen before (e.g. in LightGBM or XGBoost) and I think the skip_if_module_not_found things mean that most of the R-package test suite is effectively being skipped on CRAN.

Teach me!

from rgf.

@jameslamb Thanks a lot for your tries! I really appreciate it!

Unfortunately, I'm completely unfamiliar with R language :-( . Everything R-related was developed by @mlampros and is maintained by him too (many thanks to him for this BTW!). The only thing I've done was environment setup for R on CI services: Travis and Appveyor.

I remember, @mlampros told me that every test is indeed skipped at CRAN, because it's impossible to install required Python package (rgf_python) there for successful run (#208 (comment)). At CI side, I personally look at codecov value: if <20%, then everything was skipped: #230 (comment).

from rgf.

@jameslamb Totally agree with you! I can say even more: here, in this part Python --> C++, Python calls exe file providing a paths to previously saved data files on hard drive as arguments, instead of efficiently communicating though C API interface of dll file. But better something than nothing...

from rgf.

@jameslamb,

"if you agree I can create an issue for this and try to address it."

I'm ok with this, you can proceed.

from rgf.

@StrikerRUS hey awesome!

from rgf.

@StrikerRUS,

• for the package.R file we can not add tests because it's required mainly in order to load the python modules in R.

• I don't know if it would be feasible to add tests for the RGF_cleanup_temp_files.R file because it requires that temporary files are created first that should be deleted afterwards ( the cleanup() python function). I mean we have to keep track of the temporary directories or files that are present before the cleanup to find out if the cleanup() took actually place. Do you utilize a corresponding test in python?

• I could add tests for the Internal_class.R function. Can I do this till the end of the week? thanks.

from rgf.

@mlampros Ah, I see. I think, in Python we test only cleanup method:

I guess tests for Internal_class.R will be enough.

Sure, any time you can! Thank you very much!

from rgf.