You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ernestprovo23 (via GitHub)" <gi...@apache.org> on 2026/02/22 19:10:26 UTC
[PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
ernestprovo23 opened a new pull request, #49372:
URL: https://github.com/apache/arrow/pull/49372
### Rationale
Closes https://github.com/apache/arrow/issues/41365
When `_resolve_filesystem_and_path()` receives an S3 URI containing spaces (e.g. `s3://bucket/path with space/file`), the C++ URI parser raises `ValueError("Cannot parse URI")`. The Python code previously caught **all** `"Cannot parse URI"` errors and silently fell back to `LocalFileSystem`, producing a confusing downstream error instead of the real parsing failure.
### What changed
1. **Added `_is_likely_uri()` helper** — a Python port of the C++ `IsLikelyUri()` heuristic from `cpp/src/arrow/filesystem/path_util.cc`. It checks whether a string has a valid URI scheme prefix (2-36 chars, RFC 3986 compliant).
2. **Modified error handling in `_resolve_filesystem_and_path()`** — the `except ValueError` block now distinguishes:
- `"empty scheme"` → still falls back to `LocalFileSystem` (no scheme at all)
- `"Cannot parse URI"` + `_is_likely_uri()` returns `False` → still falls back (not a URI, just a local path)
- `"Cannot parse URI"` + `_is_likely_uri()` returns `True` → **re-raises** the error (it IS a URI, just malformed)
3. **Changed `raise e` to `raise`** — preserves the original traceback (minor style improvement).
### Tests added
- `test_is_likely_uri()` — unit tests for the heuristic covering valid schemes (`s3://`, `gs://`, `hdfs://`, `abfss://`, `grpc+https://`), local paths, Windows drives, and invalid prefixes.
- `test_resolve_filesystem_and_path_uri_with_spaces()` — verifies S3/GCS/ABFSS URIs with spaces now raise `ValueError`.
- `test_resolve_filesystem_and_path_local_with_spaces()` — verifies local paths with spaces still return `LocalFileSystem`.
### Test plan
- [x] All new tests pass (14/14)
- [x] Manual verification: `_resolve_filesystem_and_path("s3://bucket/path with space")` raises `ValueError`
- [x] Manual verification: `_resolve_filesystem_and_path("/local/path with space")` returns `LocalFileSystem`
- [ ] Existing `test_fs.py` tests pass (requires full C++ build environment)
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "ernestprovo23 (via GitHub)" <gi...@apache.org>.
ernestprovo23 commented on code in PR #49372:
URL: https://github.com/apache/arrow/pull/49372#discussion_r3853884227
##########
cpp/src/arrow/filesystem/path_util.h:
##########
@@ -159,9 +159,6 @@ std::string ToSlashes(std::string_view s);
ARROW_EXPORT
bool IsEmptyPath(std::string_view s);
-ARROW_EXPORT
-bool IsLikelyUri(std::string_view s);
-
Review Comment:
Works for me. i can handle later today
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "ernestprovo23 (via GitHub)" <gi...@apache.org>.
ernestprovo23 commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-4090084069
thanks for the detailed walkthrough on the C++/Cython approach — makes a lot more sense than reimplementing the logic in Python. working on reworking the PR to expose IsLikelyUri through the binding. will push an update soon.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "conor-f (via GitHub)" <gi...@apache.org>.
conor-f commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-4033190718
I see from the test cases that non-ASCII characters in the path are not currently accounted for. Would it be worthwhile to add this case to the added unit test cases? I have recently noticed S3 URLs with umlauts in the path being parsed as filesystem URLs and it looks like this could be the same cause!
e.g. `s3://asdf/äöü`
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #49372:
URL: https://github.com/apache/arrow/pull/49372#discussion_r3622836518
##########
cpp/src/arrow/filesystem/path_util.h:
##########
@@ -27,6 +27,10 @@
namespace arrow {
namespace fs {
+
+// Declared in arrow/filesystem/filesystem.h, defined in path_util.cc.
+ARROW_EXPORT bool IsLikelyUri(std::string_view path);
Review Comment:
Why declare it a second time? Let's choose one location and only one.
##########
cpp/src/arrow/filesystem/path_util.h:
##########
@@ -159,8 +163,7 @@ std::string ToSlashes(std::string_view s);
ARROW_EXPORT
bool IsEmptyPath(std::string_view s);
-ARROW_EXPORT
-bool IsLikelyUri(std::string_view s);
+inline bool IsLikelyUri(std::string_view s) { return ::arrow::fs::IsLikelyUri(s); }
Review Comment:
Why this?
##########
python/pyarrow/tests/test_fs.py:
##########
@@ -1725,6 +1725,134 @@ def test_filesystem_from_path_object(path):
assert path == p.resolve().absolute().as_posix()
+def test_is_likely_uri():
+ """Unit tests for the _is_likely_uri() heuristic."""
+ from pyarrow.fs import _is_likely_uri
+
+ # Valid URI schemes
+ assert _is_likely_uri("s3://bucket/key")
+ assert _is_likely_uri("gs://bucket/key")
+ assert _is_likely_uri("hdfs://host/path")
+ assert _is_likely_uri("file:///local/path")
+ assert _is_likely_uri("abfss://container@account/path")
+ assert _is_likely_uri("grpc+https://host:443")
+
+ # Only the scheme (everything before the first ':') is inspected, so
+ # non-ASCII characters in the *path* don't change the verdict.
+ assert _is_likely_uri("s3://asdf/äöü")
+ assert _is_likely_uri("s3://bucket/über/daten.parquet")
+ assert _is_likely_uri("s3://bucket/数据/file.parquet")
+
+ # Not URIs — local paths, Windows drives, empty, etc.
+ assert not _is_likely_uri("")
+ assert not _is_likely_uri("/absolute/path")
+ assert not _is_likely_uri("relative/path")
+ assert not _is_likely_uri("C:\\Users\\foo") # single-letter → drive
+ assert not _is_likely_uri("C:/Users/foo")
+ assert not _is_likely_uri("3bucket://key") # scheme starts with digit
+ assert not _is_likely_uri("-scheme://key") # scheme starts with dash
+ assert not _is_likely_uri("schéme://bucket/key") # non-ASCII in scheme
+ assert not _is_likely_uri("漢字://bucket/key") # non-ASCII in scheme
+ assert not _is_likely_uri("/tmp/äöü/data") # non-ASCII local path
+ assert not _is_likely_uri("dätä/file.parquet") # non-ASCII, no scheme
+
+
+def test_resolve_filesystem_and_path_uri_with_spaces():
+ """
+ URIs with a recognised scheme but un-encoded spaces must raise
+ ValueError — NOT silently fall back to LocalFileSystem.
+ (GH-41365)
+ """
+ from pyarrow.fs import _resolve_filesystem_and_path
+
+ # S3 URI with spaces should raise, not return LocalFileSystem
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("s3://bucket/path with space/file.parquet")
+
+ # GCS URI with spaces should also raise
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("gs://bucket/path with space/file.csv")
+
+ # abfss URI with spaces
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path(
+ "abfss://container@account/dir with space/file"
+ )
+
+
+def test_resolve_filesystem_and_path_local_with_spaces():
+ """
+ Local paths (no scheme) with spaces should still resolve to
+ LocalFileSystem — they must NOT be confused with malformed URIs.
+ """
+ from pyarrow.fs import _resolve_filesystem_and_path
+
+ # Absolute local path with spaces → LocalFileSystem
+ fs, path = _resolve_filesystem_and_path("/tmp/path with spaces/data")
+ assert isinstance(fs, LocalFileSystem)
+
+ # Non-existent absolute path → still LocalFileSystem
+ fs, path = _resolve_filesystem_and_path("/nonexistent/path")
+ assert isinstance(fs, LocalFileSystem)
+
+
+def test_resolve_filesystem_and_path_uri_with_non_ascii():
Review Comment:
Can you fold this and `test_resolve_filesystem_and_path_uri_with_spaces` together?
##########
python/pyarrow/tests/test_fs.py:
##########
@@ -1725,6 +1725,134 @@ def test_filesystem_from_path_object(path):
assert path == p.resolve().absolute().as_posix()
+def test_is_likely_uri():
+ """Unit tests for the _is_likely_uri() heuristic."""
+ from pyarrow.fs import _is_likely_uri
+
+ # Valid URI schemes
+ assert _is_likely_uri("s3://bucket/key")
+ assert _is_likely_uri("gs://bucket/key")
+ assert _is_likely_uri("hdfs://host/path")
+ assert _is_likely_uri("file:///local/path")
+ assert _is_likely_uri("abfss://container@account/path")
+ assert _is_likely_uri("grpc+https://host:443")
+
+ # Only the scheme (everything before the first ':') is inspected, so
+ # non-ASCII characters in the *path* don't change the verdict.
+ assert _is_likely_uri("s3://asdf/äöü")
+ assert _is_likely_uri("s3://bucket/über/daten.parquet")
+ assert _is_likely_uri("s3://bucket/数据/file.parquet")
+
+ # Not URIs — local paths, Windows drives, empty, etc.
+ assert not _is_likely_uri("")
+ assert not _is_likely_uri("/absolute/path")
+ assert not _is_likely_uri("relative/path")
+ assert not _is_likely_uri("C:\\Users\\foo") # single-letter → drive
+ assert not _is_likely_uri("C:/Users/foo")
+ assert not _is_likely_uri("3bucket://key") # scheme starts with digit
+ assert not _is_likely_uri("-scheme://key") # scheme starts with dash
+ assert not _is_likely_uri("schéme://bucket/key") # non-ASCII in scheme
+ assert not _is_likely_uri("漢字://bucket/key") # non-ASCII in scheme
+ assert not _is_likely_uri("/tmp/äöü/data") # non-ASCII local path
+ assert not _is_likely_uri("dätä/file.parquet") # non-ASCII, no scheme
+
+
+def test_resolve_filesystem_and_path_uri_with_spaces():
+ """
+ URIs with a recognised scheme but un-encoded spaces must raise
+ ValueError — NOT silently fall back to LocalFileSystem.
+ (GH-41365)
+ """
+ from pyarrow.fs import _resolve_filesystem_and_path
+
+ # S3 URI with spaces should raise, not return LocalFileSystem
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("s3://bucket/path with space/file.parquet")
+
+ # GCS URI with spaces should also raise
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("gs://bucket/path with space/file.csv")
+
+ # abfss URI with spaces
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path(
+ "abfss://container@account/dir with space/file"
+ )
+
+
+def test_resolve_filesystem_and_path_local_with_spaces():
+ """
+ Local paths (no scheme) with spaces should still resolve to
+ LocalFileSystem — they must NOT be confused with malformed URIs.
+ """
+ from pyarrow.fs import _resolve_filesystem_and_path
+
+ # Absolute local path with spaces → LocalFileSystem
+ fs, path = _resolve_filesystem_and_path("/tmp/path with spaces/data")
+ assert isinstance(fs, LocalFileSystem)
+
+ # Non-existent absolute path → still LocalFileSystem
+ fs, path = _resolve_filesystem_and_path("/nonexistent/path")
+ assert isinstance(fs, LocalFileSystem)
+
+
+def test_resolve_filesystem_and_path_uri_with_non_ascii():
+ """
+ URIs with a recognised scheme but un-encoded non-ASCII characters must
+ raise ValueError — NOT silently fall back to LocalFileSystem. This is
+ the same failure mode as un-encoded spaces: the URI parser rejects any
+ byte outside the RFC 3986 character set, so such paths have to be
+ percent-encoded (e.g. "s3://asdf/%C3%A4%C3%B6%C3%BC").
+ (GH-41365)
+ """
+ from pyarrow.fs import _resolve_filesystem_and_path
+
+ # S3 URI with umlauts should raise, not return LocalFileSystem
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("s3://asdf/äöü")
+
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("s3://bucket/über/daten.parquet")
+
+ # Non-Latin scripts fail the same way
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("s3://bucket/数据/file.parquet")
+
+ # GCS URI with umlauts should also raise
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("gs://bucket/äöü/x.csv")
+
+ # abfss URI with umlauts
+ with pytest.raises(ValueError, match="Cannot parse URI"):
+ _resolve_filesystem_and_path("abfss://container@account/äöü")
+
+
+def test_resolve_filesystem_and_path_local_with_non_ascii():
Review Comment:
Can you fold this and `test_resolve_filesystem_and_path_local_with_spaces` together?
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-3941547387
:warning: GitHub issue #41365 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on code in PR #49372:
URL: https://github.com/apache/arrow/pull/49372#discussion_r2838461550
##########
python/pyarrow/fs.py:
##########
@@ -82,6 +82,32 @@ def __getattr__(name):
)
+def _is_likely_uri(path):
Review Comment:
You mention on the issue:
> Working on a fix that ports the C++ IsLikelyUri() heuristic to Python and uses it in _resolve_filesystem_and_path()
Could you check if it's possible to expose `IsLikelyUri` through Python so ultimately the logic is only implemented once and is always doing the same?
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "ernestprovo23 (via GitHub)" <gi...@apache.org>.
ernestprovo23 commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-3981179196
@rok friendly ping on this when you get a chance. the ARM64 macOS 14 failure looks like a CI infra issue (missing `Paginator.h` header from the AWS SDK on the runner), not related to this change.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "ernestprovo23 (via GitHub)" <gi...@apache.org>.
ernestprovo23 commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-4486741057
@rok Pushed an update for the Cython binding issue causing the build failures.
The `cpp_string_view` cppclass in `python/pyarrow/includes/common.pxd` has the known-broken constructor declarations referenced by the [cython/cython#6651](https://github.com/cython/cython/issues/6651) comment right above it — the inner declarations are named `string_view(...)` rather than `cpp_string_view(...)` so Cython parses them as methods, not constructors. Result: `cpp_string_view(c_path)` failed with `no constructor found for C++ type 'cpp_string_view'` and then crashed the Cython compiler downstream.
Switched the `CIsLikelyUri` binding to take `const c_string&`; C++17's implicit `std::string -> std::string_view` conversion handles the call at the C++ level, so the actual C++ signature is unchanged. Verified locally that Cython now compiles `_fs.pyx` cleanly. Also rebased on main.
Two remaining CI failures look pre-existing and unrelated to this PR — flagging in case it saves you a click:
- `AMD64 Ubuntu Meson` — `cpp/src/arrow/extension/meson.build:27` references a missing `fixed_shape_tensor_test.cc`
- `ARM64 macOS 14 *` — `cpp/src/arrow/flight/transport/grpc/serialization_internal.cc:356` `nodiscard` warning treated as error
Happy to add an inline comment in `_fs.pyx` referencing `cython/cython#6651` if you'd prefer the workaround documented at the call site.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-3992535367
Hey @ernestprovo23 ! The [Paginator.h issue](https://github.com/apache/arrow/issues/49347) is unrelated.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "ernestprovo23 (via GitHub)" <gi...@apache.org>.
ernestprovo23 commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-5387625617
thanks @pitrou, all four done.
dropped the second declaration, `arrow::fs::IsLikelyUri` now lives only in `filesystem.h`. also killed the `internal::` inline forwarder you flagged... that was only there so i wouldn't have to touch callers, which isn't a good reason. the 7 in-tree call sites (mockfs, localfs, filesystem, s3fs x2, azurefs, gcsfs) call it unqualified now and they each already pull `filesystem.h` in through their own fs header, so no new includes.
folded both test pairs, the spaces and non-ascii cases are two parametrized tests now instead of four near-identical ones.
rebased on main. heads up the workflow runs are sitting at action_required so nothing's actually built on this sha yet.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on code in PR #49372:
URL: https://github.com/apache/arrow/pull/49372#discussion_r2880038402
##########
python/pyarrow/fs.py:
##########
@@ -82,6 +82,32 @@ def __getattr__(name):
)
+def _is_likely_uri(path):
Review Comment:
So I think calling the C++ function would be preferable. The change to expose the functionallity could look like:
```diff
diff --git i/cpp/src/arrow/filesystem/filesystem.h w/cpp/src/arrow/filesystem/filesystem.h
index 3a47eb62f5..85ee3ff114 100644
--- i/cpp/src/arrow/filesystem/filesystem.h
+++ w/cpp/src/arrow/filesystem/filesystem.h
@@ -23,6 +23,7 @@
#include <iosfwd>
#include <memory>
#include <string>
+#include <string_view>
#include <utility>
#include <vector>
@@ -529,6 +530,12 @@ class ARROW_EXPORT SlowFileSystem : public FileSystem {
/// The user is responsible for synchronization of calls to this function.
void EnsureFinalized();
+/// \brief Return whether a path string is likely a URI.
+///
+/// This heuristic is conservative and may return false for malformed URIs.
+ARROW_EXPORT
+bool IsLikelyUri(std::string_view path);
+
/// \defgroup filesystem-factories Functions for creating FileSystem instances
///
/// @{
diff --git i/cpp/src/arrow/filesystem/path_util.cc w/cpp/src/arrow/filesystem/path_util.cc
index dc82afd07e..cdc1807781 100644
--- i/cpp/src/arrow/filesystem/path_util.cc
+++ w/cpp/src/arrow/filesystem/path_util.cc
@@ -28,8 +28,28 @@
#include "arrow/util/uri.h"
namespace arrow {
-
namespace fs {
+
+bool IsLikelyUri(std::string_view v) {
+ if (v.empty() || v[0] == '/') {
+ return false;
+ }
+ const auto pos = v.find_first_of(':');
+ if (pos == v.npos) {
+ return false;
+ }
+ if (pos < 2) {
+ // One-letter URI schemes don't officially exist, perhaps a Windows drive letter?
+ return false;
+ }
+ if (pos > 36) {
+ // The largest IANA-registered URI scheme is "microsoft.windows.camera.multipicker"
+ // with 36 characters.
+ return false;
+ }
+ return ::arrow::util::IsValidUriScheme(v.substr(0, pos));
+}
+
namespace internal {
// XXX How does this encode Windows UNC paths?
@@ -337,26 +357,6 @@ bool IsEmptyPath(std::string_view v) {
return true;
}
diff --git i/cpp/src/arrow/filesystem/path_util.h w/cpp/src/arrow/filesystem/path_util.h
index d49d9d2efa..2f767fc7dc 100644
--- i/cpp/src/arrow/filesystem/path_util.h
+++ w/cpp/src/arrow/filesystem/path_util.h
@@ -27,6 +27,10 @@
namespace arrow {
namespace fs {
+
+ARROW_EXPORT
+bool IsLikelyUri(std::string_view s);
+
namespace internal {
constexpr char kSep = '/';
@@ -159,8 +163,7 @@ std::string ToSlashes(std::string_view s);
ARROW_EXPORT
bool IsEmptyPath(std::string_view s);
-ARROW_EXPORT
-bool IsLikelyUri(std::string_view s);
+inline bool IsLikelyUri(std::string_view s) { return ::arrow::fs::IsLikelyUri(s); }
class ARROW_EXPORT Globber {
public:
diff --git i/python/pyarrow/_fs.pyx w/python/pyarrow/_fs.pyx
index 0739b6acba..0b8ab7480f 100644
--- i/python/pyarrow/_fs.pyx
+++ w/python/pyarrow/_fs.pyx
@@ -84,6 +84,14 @@ def _file_type_to_string(ty):
return f"{ty.__class__.__name__}.{ty._name_}"
+def _is_likely_uri(path):
+ cdef c_string c_path
+ if not isinstance(path, str):
+ raise TypeError("Path must be a string")
+ c_path = tobytes(path)
+ return CIsLikelyUri(cpp_string_view(c_path))
+
+
cdef class FileInfo(_Weakrefable):
"""
FileSystem entry info.
diff --git i/python/pyarrow/fs.py w/python/pyarrow/fs.py
index 41308cd60c..b0acb97d94 100644
--- i/python/pyarrow/fs.py
+++ w/python/pyarrow/fs.py
@@ -31,6 +31,7 @@ from pyarrow._fs import ( # noqa
_MockFileSystem,
FileSystemHandler,
PyFileSystem,
+ _is_likely_uri,
_copy_files,
_copy_files_selector,
)
@@ -82,32 +83,6 @@ def __getattr__(name):
)
diff --git i/python/pyarrow/includes/libarrow_fs.pxd w/python/pyarrow/includes/libarrow_fs.pxd
index af01c47c8c..48604972d2 100644
--- i/python/pyarrow/includes/libarrow_fs.pxd
+++ w/python/pyarrow/includes/libarrow_fs.pxd
@@ -92,6 +92,7 @@ cdef extern from "arrow/filesystem/api.h" namespace "arrow::fs" nogil:
CResult[shared_ptr[CFileSystem]] CFileSystemFromUriOrPath \
"arrow::fs::FileSystemFromUriOrPath"(const c_string& uri,
c_string* out_path)
+ c_bool CIsLikelyUri "arrow::fs::IsLikelyUri"(cpp_string_view path)
cdef cppclass CFileSystemGlobalOptions \
"arrow::fs::FileSystemGlobalOptions":
diff --git i/python/pyarrow/tests/test_fs.py w/python/pyarrow/tests/test_fs.py
index 537ceb9999..31f3f8a8ca 100644
--- i/python/pyarrow/tests/test_fs.py
+++ w/python/pyarrow/tests/test_fs.py
@@ -1715,6 +1715,8 @@ def test_is_likely_uri():
assert not _is_likely_uri("C:/Users/foo")
assert not _is_likely_uri("3bucket://key") # scheme starts with digit
assert not _is_likely_uri("-scheme://key") # scheme starts with dash
+ assert not _is_likely_uri("schéme://bucket/key") # non-ASCII in scheme
+ assert not _is_likely_uri("漢字://bucket/key") # non-ASCII in scheme
```
Does that make sense?
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #49372:
URL: https://github.com/apache/arrow/pull/49372#issuecomment-3941638892
Hey @ernestprovo23, thanks for working on this and opening the PR. I'll kick off the CI run and hopefully I or someone else wil find time early next week to do a review.
--
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
Re: [PR] GH-41365: [Python] Fix S3 URI with whitespace silently falling back to LocalFileSystem [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #49372:
URL: https://github.com/apache/arrow/pull/49372#discussion_r3853766921
##########
cpp/src/arrow/filesystem/path_util.h:
##########
@@ -159,9 +159,6 @@ std::string ToSlashes(std::string_view s);
ARROW_EXPORT
bool IsEmptyPath(std::string_view s);
-ARROW_EXPORT
-bool IsLikelyUri(std::string_view s);
-
Review Comment:
Ok, I would rather the keep the declaration here, as "path utilities" is really where this function belongs IMHO.
Also, it can stay in the `internal` namespace, as that will still be visible from PyArrow AFAIK.
--
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