[SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION #23823

MichaelChirico · 2019-02-18T10:21:57Z

What changes were proposed in this pull request?

I got this warning when following the recommended approach to generating documentation:

Warning message:
roxygen2 requires Encoding: UTF-8

As can be seen in other tidyverse DESCRIPTIONs, this is standard practice

This PR adds Encoding: UTF-8 to R/pkg/DESCRIPTION

How was this patch tested?

Pass the Jenkins without warning.

I got this warning when following the recommended approach to generating documentation: ``` Warning message: roxygen2 requires Encoding: UTF-8 ``` As can be seen in [other](https://github.com/tidyverse/tidyverse/blob/master/DESCRIPTION) [`tidyverse`](https://github.com/tidyverse/dplyr/blob/master/DESCRIPTION) [`DESCRIPTION`s](https://github.com/tidyverse/readr/blob/master/DESCRIPTION), this is standard practice

felixcheung

We are *not * running the latest roxygen in the lab, there might be more of these

felixcheung · 2019-02-19T01:45:26Z

Jenkins, ok to test

HyukjinKwon · 2019-02-19T01:59:05Z

ok to test

MichaelChirico · 2019-02-19T02:14:17Z

We are *not * running the latest roxygen in the lab, there might be more of these

@felixcheung not sure if this helps/what you're getting at, but here are all the things that look like R package descriptions in Apache:

https://github.com/search?q=org%3Aapache+RoxygenNote&type=Code

felixcheung · 2019-02-19T02:19:18Z

^^ @MichaelChirico and they are all spark (except 1 mxnet)

my comment is about spark's test environment.

SparkQA · 2019-02-19T17:01:28Z

Test build #102495 has finished for PR 23823 at commit ecc964c.

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

SparkQA · 2019-02-19T17:01:55Z

Test build #4557 has finished for PR 23823 at commit ecc964c.

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

srowen · 2019-02-19T19:05:36Z

Warning messages:
1: In check_dep_version(pkg, version, compare) :
  Need roxygen2 >= 5.0.0 but loaded version is 4.1.1
2: In check_dep_version(pkg, version, compare) :
  Need roxygen2 >= 5.0.0 but loaded version is 4.1.1
++ /usr/bin/R CMD INSTALL --library=/home/jenkins/workspace/NewSparkPullRequestBuilder/R/lib /home/jenkins/workspace/NewSparkPullRequestBuilder/R/pkg/

* checking examples ... WARNING
checking a package with encoding  'UTF-8'  in an ASCII locale

Is the second one the real failure?

HyukjinKwon · 2019-02-20T02:46:43Z

I think leaving to ASCII locale actually makes more sense (for now). IIRC, Jenkins uses ASCII encoidng as default (see #22782).

felixcheung · 2019-02-20T07:09:44Z

Yes this is a real error. I’m not opposed to keeping it ASCII.

dongjoon-hyun · 2019-02-20T16:58:49Z

Given the current situation and the above advices, shall we close this PR for now, @MichaelChirico ?

MichaelChirico · 2019-02-21T00:45:15Z

I'm not quite sure I follow, is this correct:

roxygen2 requires Encoding: UTF-8 only since v6.0.1.9000 (commit here)
RoxygenNote on SparkR is only 5.0.1
Spark (and by extension SparkR) is tested only in an ASCII environment, with no plans to change at the moment

Therefore:

Leave DESCRIPTION unchanged
Add a note to DOCUMENTATION.md advising that you can ignore the warning if you're using a more recent version of roxygen2
Perhaps file a JIRA recording this, with a note to add Encoding: UTF-8 in case the test environment is ever upgraded?

felixcheung · 2019-02-21T08:43:18Z

@MichaelChirico thanks for digging through this. Is there a way to get R to run as UTF-8? that might make it possible to keep the R and Roxygen version in our environment and not create new warning there or when running newer R/Roxygen.

HyukjinKwon · 2019-03-21T23:12:59Z

UTF-8 is done as of https://issues.apache.org/jira/browse/SPARK-27177. Looks SBT masters are not done yet, and I am not sure if they are all to be done. At least PR builders are done. Let me retrigger the test and see if that fixes cc @shaneknapp FYI.

HyukjinKwon · 2019-03-21T23:13:04Z

retest this please

HyukjinKwon · 2019-03-21T23:13:48Z

@MichaelChirico, mind filing a JIRA? It's minor but think it's good to track since it's related with another JIRA.

SparkQA · 2019-03-21T23:58:00Z

Test build #103797 has finished for PR 23823 at commit ecc964c.

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

felixcheung · 2019-03-23T03:10:51Z

for merge, let's target master, and not branch-2.4

dongjoon-hyun · 2019-03-23T19:54:40Z

Retest this please.

SparkQA · 2019-03-23T20:35:34Z

Test build #103854 has finished for PR 23823 at commit ecc964c.

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

dongjoon-hyun

Thank. you, @MichaelChirico , @srowen , @HyukjinKwon , @felixcheung .

+1, LGTM. Merged to master.

dongjoon-hyun · 2019-03-23T22:26:57Z

Hi, @MichaelChirico . I need your Apache JIRA ID in order to assign it to you. What is your ID?

MichaelChirico · 2019-03-24T03:22:44Z

Hey everyone, sorry to have checked out for a bit there. Thanks @dongjoon-hyun for filing the JIRA. I commented on the created issue to link my ID to GH:

https://issues.apache.org/jira/browse/SPARK-27262?focusedCommentId=16799891&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16799891

dongjoon-hyun · 2019-03-24T04:36:15Z

Thank you, @MichaelChirico . I assigned the issue to you.

dongjoon-hyun · 2019-03-25T01:49:53Z

Hi, All.
Do you think this commit is related to the recent CRAN check errors? I saw this at spark-master-test-sbt-hadoop-2.7 profile recently. I'm wondering if CRAN check is tested on this profile only.

...
+ popd
Had CRAN check errors; see logs.
[error] running /home/jenkins/workspace/spark-master-test-sbt-hadoop-2.7/R/run-tests.sh ; received return code 255

HyukjinKwon · 2019-03-25T02:01:46Z

Yes, I think sbt master isn't handled yet per https://issues.apache.org/jira/browse/SPARK-27177?focusedCommentId=16798307&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16798307

Let's revert.

HyukjinKwon · 2019-03-25T02:07:32Z

BTW, looks after the locale is swtiched to UTF-8 in PR builder, I see weird broken characters in the Jenkins UI like:


Creating a generic function for ���lapply��� from package ���base��� in package ���SparkR���
Creating a generic function for ���Filter��� from package ���base��� in package ���SparkR���
Creating a generic function for ���alias��� from package ���stats��� in package ���SparkR���
Creating a generic function for ���substr��� from package ���base��� in package ���SparkR���
Creating a generic function for ���%in%��� from package ���base��� in package ���SparkR���
Creating a generic function for ���mean��� from package ���base��� in package ���SparkR���
Creating a generic function for ���unique��� from package ���base��� in package ���SparkR���
Creating a generic function for ���nrow��� from package ���base��� in package ���SparkR���
Creating a generic function for ���ncol��� from package ���base��� in package ���SparkR���
Creating a generic function for ���head��� from package ���utils��� in package ���SparkR���
Creating a generic function for ���factorial��� from package ���base��� in package ���SparkR���
Creating a generic function for ���atan2��� from package ���base��� in package ���SparkR���
Creating a generic function for ���ifelse��� from package ���base��� in package ���SparkR���

in R logs ... not sure if it's an issue to me or in general but thought it's good to note here before I forget.

dongjoon-hyun · 2019-03-25T04:31:12Z

I see. Thank you for reverting, @HyukjinKwon .

…ncoding to DESCRIPTION ### What changes were proposed in this pull request? This PR proposes to reenable CRAN check disabled at #27460. Given the tests #27468, seems we should also port #23823 together. ### Why are the changes needed? To check CRAN back. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? It was tested at #27468 and Jenkins should test it out. Closes #27472 from HyukjinKwon/SPARK-30737. Authored-by: HyukjinKwon <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

…ncoding to DESCRIPTION ### What changes were proposed in this pull request? This PR proposes to reenable CRAN check disabled at #27460. Given the tests #27468, seems we should also port #23823 together. ### Why are the changes needed? To check CRAN back. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? It was tested at #27468 and Jenkins should test it out. Closes #27472 from HyukjinKwon/SPARK-30737. Authored-by: HyukjinKwon <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit b95ccb1) Signed-off-by: HyukjinKwon <[email protected]>

srowen approved these changes Feb 18, 2019

View reviewed changes

felixcheung changed the title ~~Add explicit UTF-8 Encoding to DESCRIPTION~~ [R} Add explicit UTF-8 Encoding to DESCRIPTION Feb 19, 2019

felixcheung changed the title ~~[R} Add explicit UTF-8 Encoding to DESCRIPTION~~ [R] Add explicit UTF-8 Encoding to DESCRIPTION Feb 19, 2019

felixcheung reviewed Feb 19, 2019

View reviewed changes

dongjoon-hyun changed the title ~~[R] Add explicit UTF-8 Encoding to DESCRIPTION~~ [SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION Mar 23, 2019

dongjoon-hyun approved these changes Mar 23, 2019

View reviewed changes

dongjoon-hyun closed this in 240c6a4 Mar 23, 2019

HyukjinKwon mentioned this pull request Feb 6, 2020

[SPARK-30737][SPARK-27262][R][BUILD] Reenable CRAN check with UTF-8 encoding to DESCRIPTION #27472

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION #23823

[SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION #23823

MichaelChirico commented Feb 18, 2019 •

edited by dongjoon-hyun

Loading

felixcheung left a comment •

edited

Loading

felixcheung commented Feb 19, 2019

HyukjinKwon commented Feb 19, 2019

MichaelChirico commented Feb 19, 2019

felixcheung commented Feb 19, 2019

SparkQA commented Feb 19, 2019

SparkQA commented Feb 19, 2019

srowen commented Feb 19, 2019

HyukjinKwon commented Feb 20, 2019 •

edited

Loading

felixcheung commented Feb 20, 2019 via email

dongjoon-hyun commented Feb 20, 2019

MichaelChirico commented Feb 21, 2019

felixcheung commented Feb 21, 2019

HyukjinKwon commented Mar 21, 2019

HyukjinKwon commented Mar 21, 2019

HyukjinKwon commented Mar 21, 2019

SparkQA commented Mar 21, 2019

felixcheung commented Mar 23, 2019

dongjoon-hyun commented Mar 23, 2019

SparkQA commented Mar 23, 2019

dongjoon-hyun left a comment

dongjoon-hyun commented Mar 23, 2019

MichaelChirico commented Mar 24, 2019 •

edited

Loading

dongjoon-hyun commented Mar 24, 2019

dongjoon-hyun commented Mar 25, 2019

HyukjinKwon commented Mar 25, 2019

HyukjinKwon commented Mar 25, 2019

dongjoon-hyun commented Mar 25, 2019

[SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION #23823

[SPARK-27262][R] Add explicit UTF-8 Encoding to DESCRIPTION #23823

Conversation

MichaelChirico commented Feb 18, 2019 • edited by dongjoon-hyun Loading

What changes were proposed in this pull request?

How was this patch tested?

felixcheung left a comment • edited Loading

Choose a reason for hiding this comment

felixcheung commented Feb 19, 2019

HyukjinKwon commented Feb 19, 2019

MichaelChirico commented Feb 19, 2019

felixcheung commented Feb 19, 2019

SparkQA commented Feb 19, 2019

SparkQA commented Feb 19, 2019

srowen commented Feb 19, 2019

HyukjinKwon commented Feb 20, 2019 • edited Loading

felixcheung commented Feb 20, 2019 via email

dongjoon-hyun commented Feb 20, 2019

MichaelChirico commented Feb 21, 2019

felixcheung commented Feb 21, 2019

HyukjinKwon commented Mar 21, 2019

HyukjinKwon commented Mar 21, 2019

HyukjinKwon commented Mar 21, 2019

SparkQA commented Mar 21, 2019

felixcheung commented Mar 23, 2019

dongjoon-hyun commented Mar 23, 2019

SparkQA commented Mar 23, 2019

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Mar 23, 2019

MichaelChirico commented Mar 24, 2019 • edited Loading

dongjoon-hyun commented Mar 24, 2019

dongjoon-hyun commented Mar 25, 2019

HyukjinKwon commented Mar 25, 2019

HyukjinKwon commented Mar 25, 2019

dongjoon-hyun commented Mar 25, 2019

MichaelChirico commented Feb 18, 2019 •

edited by dongjoon-hyun

Loading

felixcheung left a comment •

edited

Loading

HyukjinKwon commented Feb 20, 2019 •

edited

Loading

MichaelChirico commented Mar 24, 2019 •

edited

Loading