Skip to content

[SPARK-22844][R] Adds date_trunc in R API #20031

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[SPARK-22844][R] Adds date_trunc in R API #20031

wants to merge 2 commits into from

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Dec 20, 2017

What changes were proposed in this pull request?

This PR adds date_trunc in R API as below:

> df <- createDataFrame(list(list(a = as.POSIXlt("2012-12-13 12:34:00"))))
> head(select(df, date_trunc("hour", df$a)))
  date_trunc(hour, a)
1 2012-12-13 12:00:00

How was this patch tested?

Unit tests added in R/pkg/tests/fulltests/test_sparkSQL.R.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Dec 20, 2017

cc @felixcheung, could you take a look when you have some time?

@SparkQA
Copy link

SparkQA commented Dec 20, 2017

Test build #85175 has finished for PR 20031 at commit c54abe9.

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

@SparkQA
Copy link

SparkQA commented Dec 20, 2017

Test build #85177 has finished for PR 20031 at commit 1c3e956.

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

dongjoon-hyun
dongjoon-hyun approved these changes Dec 20, 2017
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM

felixcheung
felixcheung approved these changes Dec 21, 2017
View reviewed changes
R/pkg/R/functions.R Outdated
#' to use to specify the truncation method. For example, "year", "yyyy", "yy" for
#' truncate by year, or "month", "mon", "mm" for truncate by month.
#' Column \code{x} to DateType or TimestampType.
#'
Copy link
Member

@felixcheung felixcheung Dec 21, 2017

Choose a reason for hiding this comment

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

I'm pretty sure these whitespace will be stripped by roxygen2.
use item if you think that makes sense, otherwise it might be better to keep as a single paragraph

Copy link
Member Author

@HyukjinKwon HyukjinKwon Dec 22, 2017

Choose a reason for hiding this comment

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

Yup, I am fine with fixing. Will check and fix late tomorrow.

HyukjinKwon
HyukjinKwon commented Dec 22, 2017
View reviewed changes
R/pkg/R/functions.R
#' "mm" for truncate by month.
#' \item \code{date_trunc}: it is similar with \code{trunc}'s but additionally
#' supports "day", "dd", "second", "minute", "hour", "week" and "quarter".
#' }
Copy link
Member Author

@HyukjinKwon HyukjinKwon Dec 22, 2017

Choose a reason for hiding this comment

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

The doc looks like this:

2017-12-22 9 01 30

Copy link
Member Author

@HyukjinKwon HyukjinKwon Dec 22, 2017

Choose a reason for hiding this comment

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

I am fine with a single paragraph too. Please let me know if you have a preference @felixcheung.

@SparkQA
Copy link

SparkQA commented Dec 22, 2017

Test build #85308 has finished for PR 20031 at commit 6aa835e.

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

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Dec 23, 2017

Thanks for review and approval, @dongjoon-hyun and @felixcheung.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Dec 23, 2017

Merged to master.

@asfgit asfgit closed this in aeb45df Dec 23, 2017
@HyukjinKwon HyukjinKwon deleted the r-datetrunc branch January 2, 2018 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixcheung felixcheung felixcheung approved these changes

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HyukjinKwon @SparkQA @felixcheung @dongjoon-hyun