[SPARK-11263][SPARKR] lintr Throws Warnings on Commented Code in Documentation

[felixcheung]

Clean out hundreds of style: Commented code should be removed. from lintr

Like these:

/opt/spark-1.6.0-bin-hadoop2.6/R/pkg/R/DataFrame.R:513:3: style: Commented code should be removed.
# sc <- sparkR.init()
  ^~~~~~~~~~~~~~~~~~~
/opt/spark-1.6.0-bin-hadoop2.6/R/pkg/R/DataFrame.R:514:3: style: Commented code should be removed.
# sqlContext <- sparkRSQL.init(sc)
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/spark-1.6.0-bin-hadoop2.6/R/pkg/R/DataFrame.R:515:3: style: Commented code should be removed.
# path <- "path/to/file.json"
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~

tried without @export or @Rdname, neither work
instead, added this #' @noRd to suppress .Rd file generation

also updated @family for DataFrame functions for longer descriptive text instead of dataframe_funcs

this covers most of 'Commented code' but I left out a few that looks legitimate.

[SparkQA]

Test build #45012 has finished for PR 9463 at commit 4b136b3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[yu-iskw]

LGTM
@noRd is a good idea. We still have other commented code warnings which aren't realted to docs, right?

[yu-iskw]

In my research, lintr have a bug of commented_code_linter.
I just reported the bug to the lintr community.
r-lib/lintr#118

R/RDD.R:754:20: style: Commented code should be removed.
#' take(rdd, 2L) # list(1, 2)
                   ^~~~~~~~~~

[felixcheung]

@yu-iskw yes, there are roughly a dozen "commented code" warnings left but they are mostly false positives, like the example you quoted.
At this point I'd rather not change them for the sake of being clean. Besides, we still have some ways to go to be completely lintr clean (with the chosen settings)

[shivaram]

@felixcheung Thanks for taking a shot at this. Can we remove the @export from these functions as well ? They are not exported in the public API so it seems thats not needed ?

[felixcheung]

Done. they won't do anything because of @noRd

[felixcheung]

@yu-iskw btw, these are two other false positives that you might want to follow up with lintr:

R/pkg/inst/tests/test_sparkSQL.R:907:53: style: Commented code should be removed.
  expect_equal(collect(select(df2, lpad(df2$a, 8, "#")))[1, 1], "###aaads")
                                                    ^~~~~~~~~~~~~~~~~~~~~~~

R/pkg/R/RDD.R:228:63: style: Commented code should be removed.
#' http://spark.apache.org/docs/latest/programming-guide.html#rdd-persistence.
                                                              ^~~~~~~~~~~~~~~~

[SparkQA]

Test build #45092 has finished for PR 9463 at commit 037f0df.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[felixcheung]

@shivaram ?

[felixcheung]

@shivaram ?

[shivaram]

Sorry this slipped through. Will look at this tonight

[felixcheung]

Could we push this, at least to master? that would really help dev running lintr for a much more manageable volume of warnings/errors

[shivaram]

Can you remove the @family from here ? Even though we have a noRd here it ends up generating the toRDD method as a link in the other DataFrame functions section

[shivaram]

We might also need to remove the @rdname DataFrame here as it shows up in the DataFrame documentation page.

[shivaram]

Sorry for the delay. I found one minor problem with toRDD -- otherwise LGTM.

[felixcheung]

thanks for catching that, I did some test I suspect they are caused by generics.R, so removing those.

[SparkQA]

Test build #45801 has finished for PR 9463 at commit f98bea2.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #45803 has finished for PR 9463 at commit c8c0f2e.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[shivaram]

Thanks @felixcheung - Looks good now. Merging this.