ianmcook commented on a change in pull request #9800:
URL: https://github.com/apache/arrow/pull/9800#discussion_r607428950



##########
File path: r/R/filesystem.R
##########
@@ -273,33 +273,53 @@ FileSystem$from_uri <- function(uri) {
   fs___FileSystemFromUri(uri)
 }
 
-get_path_and_filesystem <- function(x, filesystem = NULL) {
+get_paths_and_filesystem <- function(x, filesystem = NULL) {
   # Wrapper around FileSystem$from_uri that handles local paths
   # and an optional explicit filesystem
   if (inherits(x, "SubTreeFileSystem")) {
     return(list(fs = x$base_fs, path = x$base_path))
   }
-  assert_that(is.string(x))
-  if (is_url(x)) {
+  assert_that(is.character(x))
+  are_urls <- are_urls(x)
+  if (any(are_urls)) {
+    if (!all(are_urls)) {
+      stop("Vectors of mixed paths and URIs are not supported", call. = FALSE)

Review comment:
       The function in the C++ library that this ultimately calls to generate a 
dataset from multiple files expects all those files to be in the same 
filesystem.
   
   I suppose in the case where the user mixed `file://` URIs and plain file 
paths with no protocol, we could recognize that they both point to the local 
file system, but that would require extra code and I don't think it would add 
any practical value




-- 
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.

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


Reply via email to