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

[SPARK-20889][SparkR] Grouped documentation for NONAGGREGATE column methods #18422

Closed
wants to merge 7 commits into from

Conversation

actuaryzhang
Copy link
Contributor

What changes were proposed in this pull request?

Grouped documentation for nonaggregate column methods.

@SparkQA
Copy link

SparkQA commented Jun 26, 2017

Test build #78643 has finished for PR 18422 at commit d2a7ca8.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jun 26, 2017

Test build #78646 has finished for PR 18422 at commit d2a7ca8.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 26, 2017

Test build #78655 has finished for PR 18422 at commit 3057856.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jun 26, 2017

Test build #78662 has finished for PR 18422 at commit 3057856.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 27, 2017

Test build #78665 has finished for PR 18422 at commit 4e77d7b.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jun 27, 2017

Test build #78668 has finished for PR 18422 at commit 4e77d7b.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

2 similar comments
@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

@felixcheung
Copy link
Member

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jun 27, 2017

Test build #78713 has finished for PR 18422 at commit 4e77d7b.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 28, 2017

Test build #78819 has finished for PR 18422 at commit b72bf9c.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 28, 2017

Test build #78817 has finished for PR 18422 at commit 83ecb98.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@actuaryzhang
Copy link
Contributor Author

jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jun 28, 2017

Test build #78824 has finished for PR 18422 at commit b72bf9c.

  • This patch fails due to an unknown error code, -10.
  • This patch merges cleanly.
  • This patch adds no public classes.

NULL

#' @details
#' \code{lit}: A new \linkS4class{Column} is created to represent the literal value.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this format is actually kinda weird. let's fix it? I don't think we need to link to Column
(yes, I think I added this...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

#' head(select(df, input_file_name()))
#' }
#' \dontrun{
#' tmp <- read.text("README.md")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why rename to tmp though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid overwriting the dataframe example df used throughout the doc.

#'
#' @param x Column to compute on.
#' @details
#' \code{is.nan}: Alias for \link{isnan}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roxygen does this by text order, I think - doesn't it make this go first, before isnan? perhaps we swap the order of code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, swapped the order.

#' tmp <- mutate(df, v1 = lit(df$mpg), v2 = lit("x"), v3 = lit("2015-01-01"),
#' v4 = negate(df$mpg), v5 = expr('length(model)'),
#' v6 = greatest(df$vs, df$am), v7 = least(df$vs, df$am),
#' v8 = column("mpg"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there example for

nanvl(df$c, x)
coalesce(df$c, df$d, df$e) 

that I've missed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See L2796.

#' @param x Column to compute on. In \code{lit}, it is a literal value or a Column.
#' In \code{monotonically_increasing_id}, it should be empty.
#' @param y Column to compute on.
#' @param ... additional argument(s). In \code{expr}, it contains an expression character
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In \code{expr}, it contains an expression character - this isn't quite right actually - it's in x in expr, not as ... parameter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and so in all other cases in this group, ... is expected for other columns. perhaps we can say additional Columns

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, thanks for catching this.

#'
#' @param x a literal value or a Column.
#' @param x Column to compute on. In \code{lit}, it is a literal value or a Column.
#' In \code{monotonically_increasing_id}, it should be empty.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and same for input_file_name - btw, should be empty might be a bit confusing? how about In ......., Should be used with no argument.? or omitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was just copying from the old doc.
I now remove this and add The method should be used with no argument. to the two individual methods.

@SparkQA
Copy link

SparkQA commented Jun 29, 2017

Test build #78855 has finished for PR 18422 at commit aff832e.

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

#' head(tmp)
#' tmp <- mutate(tmp, ind_na1 = is.nan(tmp$mpg_na), ind_na2 = isnan(tmp$mpg_na))
#' head(select(tmp, coalesce(tmp$mpg_na, tmp$mpg)))
#' head(select(tmp, nanvl(tmp$mpg_na, tmp$hp)))}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixcheung Examples for coalesce and nanvl are here.

@SparkQA
Copy link

SparkQA commented Jun 29, 2017

Test build #78871 has finished for PR 18422 at commit 1d0989a.

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

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@felixcheung
Copy link
Member

merged to master

@asfgit asfgit closed this in a2d5623 Jun 29, 2017
robert3005 pushed a commit to palantir/spark that referenced this pull request Jun 29, 2017
…ethods

## What changes were proposed in this pull request?

Grouped documentation for nonaggregate column methods.

Author: actuaryzhang <actuaryzhang10@gmail.com>
Author: Wayne Zhang <actuaryzhang10@gmail.com>

Closes apache#18422 from actuaryzhang/sparkRDocNonAgg.
@actuaryzhang actuaryzhang deleted the sparkRDocNonAgg branch June 30, 2017 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants