You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by MichaelChirico <gi...@git.apache.org> on 2018/09/09 05:12:35 UTC
[GitHub] spark pull request #22370: don't link to deprecated function
GitHub user MichaelChirico opened a pull request:
https://github.com/apache/spark/pull/22370
don't link to deprecated function
Seems misleading to (without qualification) link to a deprecated function
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MichaelChirico/spark patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22370.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22370
----
commit e8b0d6333a1c09787e1c37a6f91eb895dee8fa72
Author: Michael Chirico <mi...@...>
Date: 2018-09-09T05:12:27Z
don't link to deprecated function
Seems misleading to (without qualification) link to a deprecated function
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico closed the pull request at:
https://github.com/apache/spark/pull/22370
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216229836
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
If we should fix, yea, better fix them all. @felixcheung has a better insight in R side. You batter wait for his opinion.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico commented on the issue:
https://github.com/apache/spark/pull/22370
@felixcheung I disagree... what's the point of deprecation if it's going to keep being considered as a co-equal function in the eyes of documentation?
If the function is being deprecated, it should be "soft-blacklisted" -- documentation still available (but clearly marked) for users of "legacy" code where its usage still survives, but I don't see any argument for even letting new users find out a deprecated function exists.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/22370
I don’t feel strongly either way.
I do think this is very minor since there are still many other ways to the doc page for createExternalTable (eg the index page) or via ? search within R etc. I am not sure how much difference this would make and we have already a) code spewing out warning when called b) clearly documented as Deprecated on the doc page title.
Should you find other deprecation that is not documentation we should be gladly having your help
to document it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216539411
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
`registerTempTable` is because of the `@family` tag, so it's a bit different.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216222094
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
I don't see it here (nothing pointing to `sparkR.init`/`sparkRHive.init`/`sparkRSQL.init`):
https://spark.apache.org/docs/latest/api/R/sparkR.session.html
or here (nothing pointing to `dropTempTable`):
https://spark.apache.org/docs/latest/api/R/dropTempView.html
But I do see it here (points to `registerTempTable`):
https://spark.apache.org/docs/latest/api/R/createOrReplaceTempView.html
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22370
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216222159
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
Shall I amend the PR to fix the `registerTempTable` page as well?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22370
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216217235
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
One thing we should check for such things is, if it happens globally in the documentation or not BTW.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico commented on the issue:
https://github.com/apache/spark/pull/22370
@felixcheung thanks for the feedback, yes, I axed `registerTempTable` from the `SparkDataFrame `@family`.
Sorry I botched an attempt to try and rename the branch of this PR (apparently not possible to overwrite this PR with a new branch name); PR re-filed as #22393
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22370: don't link to deprecated function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22370
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22370: don't link to deprecated function
Posted by MichaelChirico <gi...@git.apache.org>.
Github user MichaelChirico commented on a diff in the pull request:
https://github.com/apache/spark/pull/22370#discussion_r216220254
--- Diff: R/pkg/R/catalog.R ---
@@ -69,7 +69,6 @@ createExternalTable <- function(x, ...) {
#' @param ... additional named parameters as options for the data source.
#' @return A SparkDataFrame.
#' @rdname createTable
-#' @seealso \link{createExternalTable}
--- End diff --
Agree...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org