You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/01/03 13:29:34 UTC

[GitHub] [arrow] paleolimbot opened a new pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

paleolimbot opened a new pull request #12062:
URL: https://github.com/apache/arrow/pull/12062


   This PR updates the pointer logic that changed in #12011 (ARROW-15169) to make sure that users can use the arrow R package to communicate with pyarrow that hasn't been upgraded yet.
   
   Should work with:
   
   ``` bash
   pip3 install pyarrow
   ```
   
   ``` r
   pa <- reticulate::import("pyarrow", convert = FALSE)
   py <- pa$array(c(1, 2, 3))
   reticulate::py_to_r(py)
   #> [
   #>   1,
   #>   2,
   #>   3
   #> ]
   ```
   
   and:
   
   ``` bash
   pip3 install pyarrow --extra-index-url https://repo.fury.io/arrow-nightlies/ --pre --upgrade
   ```
   
   ``` r
   pa <- reticulate::import("pyarrow", convert = FALSE)
   py <- pa$array(c(1, 2, 3))
   reticulate::py_to_r(py)
   #> [
   #>   1,
   #>   2,
   #>   3
   #> ]
   ```
   
   ...but we don't have a good way to test against the old pyarrow version in our tests (unless @jonkeane can think of one!)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] paleolimbot commented on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1004116153


   I think there's a JIRA ticket for testing on Windows (if there isn't, I'll make one). Maybe that's a good ticket to use for adding a few tests for whatever `pip3 install pyarrow` will get you (as opposed to nightly)?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] paleolimbot commented on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1009119233


   Ticket for testing on Windows: ARROW-15242


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane closed pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #12062:
URL: https://github.com/apache/arrow/pull/12062


   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot commented on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1011396587


   Benchmark runs are scheduled for baseline = b1a3e8cd8e67292c8c2f4f87ed29d685daae8215 and contender = 0bce4404638f903a308f7f4afd7adc14ab164e08. 0bce4404638f903a308f7f4afd7adc14ab164e08 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/567d49bf7fb848b8b1e4b1e12a4dceb2...ebafd7c51c31473ba38c71a7b0d20305/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b0b280a1b14c40c1aea45d52e2445d29...40f7d4f4ada34a38919967fd1249d9d3/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/58e70a85435d42b68173c467c2eeb650...a72e0cee82c64848a676605dc2b688df/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on a change in pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#discussion_r781524311



##########
File path: r/R/python.R
##########
@@ -224,3 +306,14 @@ install_pyarrow <- function(envname = NULL, nightly = FALSE, ...) {
     reticulate::py_install("pyarrow", envname = envname, ...)
   }
 }
+
+pyarrow_version <- function() {
+  pa <- reticulate::import("pyarrow")
+  version_string <- pa$`__version__`
+  # remove trailing .devXXX because it won't work with package_version()
+  package_version(gsub("\\.dev.*?$", "", version_string))
+}
+
+pyarrow_version_pointers_changed <- function() {
+  "7.0.0"
+}

Review comment:
       Is there any reason you have this as a function returning a version instead of simply `pyarrow_version_pointers_changed <- "7.0.0"`?




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1011396587


   Benchmark runs are scheduled for baseline = b1a3e8cd8e67292c8c2f4f87ed29d685daae8215 and contender = 0bce4404638f903a308f7f4afd7adc14ab164e08. 0bce4404638f903a308f7f4afd7adc14ab164e08 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/567d49bf7fb848b8b1e4b1e12a4dceb2...ebafd7c51c31473ba38c71a7b0d20305/)
   [Failed :arrow_down:0.45% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b0b280a1b14c40c1aea45d52e2445d29...40f7d4f4ada34a38919967fd1249d9d3/)
   [Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/58e70a85435d42b68173c467c2eeb650...a72e0cee82c64848a676605dc2b688df/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1011396587


   Benchmark runs are scheduled for baseline = b1a3e8cd8e67292c8c2f4f87ed29d685daae8215 and contender = 0bce4404638f903a308f7f4afd7adc14ab164e08. 0bce4404638f903a308f7f4afd7adc14ab164e08 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/567d49bf7fb848b8b1e4b1e12a4dceb2...ebafd7c51c31473ba38c71a7b0d20305/)
   [Failed :arrow_down:0.45% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b0b280a1b14c40c1aea45d52e2445d29...40f7d4f4ada34a38919967fd1249d9d3/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/58e70a85435d42b68173c467c2eeb650...a72e0cee82c64848a676605dc2b688df/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] jonkeane commented on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1004105805


   > ...but we don't have a good way to test against the old pyarrow version in our tests (unless @jonkeane can think of one!)
   
   I'm not sure we need to test the backwards compatibility necessarily, but I believe one could install an older version of pyarrow with something like `reticulate::py_install("pyarrow==6.0.0", envname = envmane)` which _should_ install a pinned older version of pyarrow to test against


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1004093200






-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] pitrou commented on a change in pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#discussion_r778163085



##########
File path: r/R/python.R
##########
@@ -181,9 +257,15 @@ r_to_py.RecordBatchReader <- function(x, convert = FALSE) {
   pa <- reticulate::import("pyarrow", convert = FALSE)
   x$export_to_c(stream_ptr)
   # TODO: handle subclasses of RecordBatchReader?
-  out <- pa$lib$RecordBatchReader$`_import_from_c`(
-    stream_ptr
-  )
+
+  if (pyarrow_version() >= pyarrow_version_pointers_changed()) {

Review comment:
       Will this actually compare version strings, such that "10.0.0" is smaller than "7.0.0"?
   




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] paleolimbot commented on a change in pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on a change in pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#discussion_r781667565



##########
File path: r/R/python.R
##########
@@ -224,3 +306,14 @@ install_pyarrow <- function(envname = NULL, nightly = FALSE, ...) {
     reticulate::py_install("pyarrow", envname = envname, ...)
   }
 }
+
+pyarrow_version <- function() {
+  pa <- reticulate::import("pyarrow")
+  version_string <- pa$`__version__`
+  # remove trailing .devXXX because it won't work with package_version()
+  package_version(gsub("\\.dev.*?$", "", version_string))
+}
+
+pyarrow_version_pointers_changed <- function() {
+  "7.0.0"
+}

Review comment:
       Thanks for the catch...definitely better as a variable!




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#issuecomment-1011396587


   Benchmark runs are scheduled for baseline = b1a3e8cd8e67292c8c2f4f87ed29d685daae8215 and contender = 0bce4404638f903a308f7f4afd7adc14ab164e08. 0bce4404638f903a308f7f4afd7adc14ab164e08 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/567d49bf7fb848b8b1e4b1e12a4dceb2...ebafd7c51c31473ba38c71a7b0d20305/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b0b280a1b14c40c1aea45d52e2445d29...40f7d4f4ada34a38919967fd1249d9d3/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/58e70a85435d42b68173c467c2eeb650...a72e0cee82c64848a676605dc2b688df/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] paleolimbot commented on a change in pull request #12062: ARROW-15173: [R] Provide backward compatibility for bridge to older versions of pyarrow

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on a change in pull request #12062:
URL: https://github.com/apache/arrow/pull/12062#discussion_r778225004



##########
File path: r/R/python.R
##########
@@ -181,9 +257,15 @@ r_to_py.RecordBatchReader <- function(x, convert = FALSE) {
   pa <- reticulate::import("pyarrow", convert = FALSE)
   x$export_to_c(stream_ptr)
   # TODO: handle subclasses of RecordBatchReader?
-  out <- pa$lib$RecordBatchReader$`_import_from_c`(
-    stream_ptr
-  )
+
+  if (pyarrow_version() >= pyarrow_version_pointers_changed()) {

Review comment:
       It does!
   
   ``` r
   package_version("10.0.0") > "7.0.0"
   #> [1] TRUE
   ```




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org