Skip to content

Fix filesystem #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

Fix filesystem #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e8c3a43
fix file system with env variables to set scheme and net loc if not s…
Aug 6, 2025
ae6cc0d
add test
Aug 6, 2025
cd939c7
fix linting
Aug 6, 2025
731075a
use catalog env configs and update to use default scheme and netloc f…
Aug 25, 2025
9bfb7e8
fix linter
Aug 25, 2025
1a75ab6
address comments
Aug 26, 2025
9387d6e
pr comments
Aug 27, 2025
80176e8
pr comments update comment
Aug 27, 2025
13001c8
simplify code, return to previous structure
Aug 28, 2025
35f4cdc
move variable usage closer to usage
Aug 28, 2025
2b83b79
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions pyiceberg/io/pyarrow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,17 @@ def __init__(self, properties: Properties = EMPTY_DICT):
super().__init__(properties=properties)

@staticmethod
def parse_location(location: str) -> Tuple[str, str, str]:
"""Return the path without the scheme."""
def parse_location(location: str, properties: Properties = EMPTY_DICT) -> Tuple[str, str, str]:
"""Return (scheme, netloc, path) for the given location.

Uses DEFAULT_SCHEME and DEFAULT_NETLOC if scheme/netloc are missing.
"""
uri = urlparse(location)

if not uri.scheme:
return "file", uri.netloc, os.path.abspath(location)
default_scheme = properties.get("DEFAULT_SCHEME", "file")
default_netloc = properties.get("DEFAULT_NETLOC", "")
return default_scheme, default_netloc, os.path.abspath(location)
elif uri.scheme in ("hdfs", "viewfs"):
return uri.scheme, uri.netloc, uri.path
else:
Expand Down Expand Up @@ -614,7 +620,7 @@ def new_input(self, location: str) -> PyArrowFile:
Returns:
PyArrowFile: A PyArrowFile instance for the given location.
"""
scheme, netloc, path = self.parse_location(location)
scheme, netloc, path = self.parse_location(location, self.properties)
return PyArrowFile(
fs=self.fs_by_scheme(scheme, netloc),
location=location,
Expand All @@ -631,7 +637,7 @@ def new_output(self, location: str) -> PyArrowFile:
Returns:
PyArrowFile: A PyArrowFile instance for the given location.
"""
scheme, netloc, path = self.parse_location(location)
scheme, netloc, path = self.parse_location(location, self.properties)
return PyArrowFile(
fs=self.fs_by_scheme(scheme, netloc),
location=location,
Expand All @@ -652,7 +658,7 @@ def delete(self, location: Union[str, InputFile, OutputFile]) -> None:
an AWS error code 15.
"""
str_location = location.location if isinstance(location, (InputFile, OutputFile)) else location
scheme, netloc, path = self.parse_location(str_location)
scheme, netloc, path = self.parse_location(str_location, self.properties)
fs = self.fs_by_scheme(scheme, netloc)

try:
Expand Down
26 changes: 26 additions & 0 deletions tests/io/test_pyarrow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2785,3 +2785,29 @@ def _expected_batch(unit: str) -> pa.RecordBatch:
)

assert _expected_batch("ns" if format_version > 2 else "us").equals(actual_result)


def test_parse_location_defaults() -> None:
"""Test that parse_location uses defaults."""

from pyiceberg.io.pyarrow import PyArrowFileIO

# if no default scheme or netloc is provided, use file scheme and empty netloc
scheme, netloc, path = PyArrowFileIO.parse_location("/foo/bar")
assert scheme == "file"
assert netloc == ""
assert path == "/foo/bar"

scheme, netloc, path = PyArrowFileIO.parse_location(
"/foo/bar", properties={"DEFAULT_SCHEME": "scheme", "DEFAULT_NETLOC": "netloc:8000"}
)
assert scheme == "scheme"
assert netloc == "netloc:8000"
assert path == "/foo/bar"

scheme, netloc, path = PyArrowFileIO.parse_location(
"/foo/bar", properties={"DEFAULT_SCHEME": "hdfs", "DEFAULT_NETLOC": "netloc:8000"}
)
assert scheme == "hdfs"
Comment on lines +2788 to +2811
Copy link

@gemini-code-assist gemini-code-assist bot Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test assertions for path are not platform-independent. The parse_location function uses os.path.abspath(location) for schemeless paths, but the test hardcodes the expected path as "/foo/bar". This will work on Unix-like systems but fail on Windows where the absolute path would be different (e.g., C:\\foo\\bar).

To make the test robust across different operating systems, you should compare the result with os.path.abspath().

Additionally, the import from pyiceberg.io.pyarrow import PyArrowFileIO is inside the test function. It's better to move it to the top of the file to follow the common style in this project. I have removed it in the suggestion below.

def test_parse_location_defaults() -> None:
    """Test that parse_location uses defaults."""

    location = "/foo/bar"
    expected_path = os.path.abspath(location)

    # if no default scheme or netloc is provided, use file scheme and empty netloc
    scheme, netloc, path = PyArrowFileIO.parse_location(location)
    assert scheme == "file"
    assert netloc == ""
    assert path == expected_path

    scheme, netloc, path = PyArrowFileIO.parse_location(
        location, properties={"DEFAULT_SCHEME": "scheme", "DEFAULT_NETLOC": "netloc:8000"}
    )
    assert scheme == "scheme"
    assert netloc == "netloc:8000"
    assert path == expected_path

    scheme, netloc, path = PyArrowFileIO.parse_location(
        location, properties={"DEFAULT_SCHEME": "hdfs", "DEFAULT_NETLOC": "netloc:8000"}
    )
    assert scheme == "hdfs"
    assert netloc == "netloc:8000"
    assert path == expected_path

assert netloc == "netloc:8000"
assert path == "/foo/bar"