You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/03 08:47:06 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request, #36758: [SPARK-39372][INFRA][R] Update R version to 4.2.0 in AppVeyor
HyukjinKwon opened a new pull request, #36758:
URL: https://github.com/apache/spark/pull/36758
### What changes were proposed in this pull request?
This PR updates AppVeyor to use the latest R version 4.2.0.
### Why are the changes needed?
To test the latest R. R community tends to use the latest versions aggresively.
### Does this PR introduce _any_ user-facing change?
No, dev-only
### How was this patch tested?
CI in this PR should test it out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][INFRA][R] Update R version to 4.2.0 in AppVeyor
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888810253
##########
dev/appveyor-install-dependencies.ps1:
##########
@@ -129,7 +129,7 @@ $env:PATH = "$env:HADOOP_HOME\bin;" + $env:PATH
Pop-Location
# ========================== R
-$rVer = "4.0.2"
+$rVer = "4.2.0"
$rToolsVer = "4.0.2"
Review Comment:
While the tests passed, it failed to download RTools 4.2.0. I reverted the RTools upgrade here for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888911462
##########
R/pkg/R/serialize.R:
##########
@@ -58,7 +58,12 @@ writeObject <- function(con, object, writeType = TRUE) {
# Checking types is needed here, since 'is.na' only handles atomic vectors,
# lists and pairlists
if (type %in% c("integer", "character", "logical", "double", "numeric")) {
- if (is.na(object)) {
+ if (is.na(object[[1]])) {
Review Comment:
**R 4.1 and below:**
```
Warning in if (is.na(c(1, 2))) print("abc") :
the condition has length > 1 and only the first element will be used
```
**R 4.2+:**
```
Error in if (is.na(c(1, 2))) print("abc") : the condition has length > 1
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36758:
URL: https://github.com/apache/spark/pull/36758#issuecomment-1145940475
cc @felixcheung @shivaram @viirya too FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36758:
URL: https://github.com/apache/spark/pull/36758#issuecomment-1145930588
Tests should pass now ..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #36758: [SPARK-39372][R] Support R 4.2.0
URL: https://github.com/apache/spark/pull/36758
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][INFRA][R] Update R version to 4.2.0 in AppVeyor
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888810253
##########
dev/appveyor-install-dependencies.ps1:
##########
@@ -129,7 +129,7 @@ $env:PATH = "$env:HADOOP_HOME\bin;" + $env:PATH
Pop-Location
# ========================== R
-$rVer = "4.0.2"
+$rVer = "4.2.0"
$rToolsVer = "4.0.2"
Review Comment:
While the tests passed (one test failed but seems flaky), it failed to download RTools 4.2.0. I reverted the RTools upgrade here for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888920686
##########
R/pkg/R/serialize.R:
##########
@@ -58,7 +58,12 @@ writeObject <- function(con, object, writeType = TRUE) {
# Checking types is needed here, since 'is.na' only handles atomic vectors,
# lists and pairlists
if (type %in% c("integer", "character", "logical", "double", "numeric")) {
- if (is.na(object)) {
+ if (is.na(object[[1]])) {
+ # Uses the first element for now to keep the behavior same as R before
+ # 4.2.0. This is wrong because we should differenciate c(NA) from a
+ # single NA as the former means array(null) and the latter means null
+ # in Spark SQL. However, it requires non-trivial comparison to distinguish
Review Comment:
e.g.) we should check if the input is vector, list, array, etc, which is exactly being done at `getSerdeType`. However, this comparison here (up to my best knowledge) is a shortcut to avoid the overhead from `getSerdeType`. So, I just decided to leave it as is for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36758:
URL: https://github.com/apache/spark/pull/36758#issuecomment-1146426846
Thanks!!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888858660
##########
R/pkg/R/mllib_classification.R:
##########
@@ -331,7 +331,7 @@ setMethod("spark.logit", signature(data = "SparkDataFrame", formula = "formula")
}
if (!is.null(upperBoundsOnCoefficients)) {
- if (class(upperBoundsOnCoefficients) != "matrix") {
+ if (is.matrix(upperBoundsOnCoefficients)) {
Review Comment:
```suggestion
if (!is.matrix(upperBoundsOnCoefficients)) {
```
##########
R/pkg/R/mllib_classification.R:
##########
@@ -322,7 +322,7 @@ setMethod("spark.logit", signature(data = "SparkDataFrame", formula = "formula")
}
if (!is.null(lowerBoundsOnCoefficients)) {
- if (class(lowerBoundsOnCoefficients) != "matrix") {
+ if (is.matrix(lowerBoundsOnCoefficients)) {
Review Comment:
```suggestion
if (!is.matrix(lowerBoundsOnCoefficients)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36758: [SPARK-39372][R] Support R 4.2.0
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36758:
URL: https://github.com/apache/spark/pull/36758#discussion_r888810253
##########
dev/appveyor-install-dependencies.ps1:
##########
@@ -129,7 +129,7 @@ $env:PATH = "$env:HADOOP_HOME\bin;" + $env:PATH
Pop-Location
# ========================== R
-$rVer = "4.0.2"
+$rVer = "4.2.0"
$rToolsVer = "4.0.2"
Review Comment:
While the tests passed, it failed to download RTools 4.2.0. I reverted the RTools upgrade here for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org