Skip to content

Commit

Permalink
Pass through resolve_path when the path is not accessible (cctbx#772)
Browse files Browse the repository at this point in the history
* Resolve path only if accessible, otherwise return unchanged. Fixes cctbx#771
* Given that we're already monkeypatching in the test, use this to also fake out os.path.exists to get the old behaviour for the test. Add a new assert to test the new behaviour first.
  • Loading branch information
dagewa authored Dec 10, 2024
1 parent edb60e4 commit d7be9b2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
1 change: 1 addition & 0 deletions newsfragments/772.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not try to resolve file paths if they are not accessible.
14 changes: 9 additions & 5 deletions src/dxtbx/serialize/filename.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@ def resolve_path(path, directory=None):
directory (Optional[str]): The local path to resolve relative links
Returns:
str: The absolute path to the file to read
str: The absolute path to the file to read if accessible, otherwise
return the original path as provided
"""
if not path:
return ""
path = os.path.expanduser(os.path.expandvars(path))
if directory and not os.path.isabs(path):
path = os.path.join(directory, path)
return os.path.abspath(path)
trial_path = os.path.expanduser(os.path.expandvars(path))
if directory and not os.path.isabs(trial_path):
trial_path = os.path.join(directory, trial_path)
if os.path.exists(trial_path):
return os.path.abspath(trial_path)
else:
return path
23 changes: 19 additions & 4 deletions tests/serialize/test_filename.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,30 @@
from __future__ import annotations

import os
import uuid

from dxtbx.serialize.filename import resolve_path


def test_resolve_path(monkeypatch):
# Resolve path
def alwaystrue(path):
return True

# Set an environment variable
monkeypatch.setenv("HELLO_WORLD", "EXPANDED")
new_path = os.path.join("~", "$HELLO_WORLD", "path")

# First try a path that does not exist
filename = str(uuid.uuid4())
new_path = os.path.join("~", "$HELLO_WORLD", filename)
path = resolve_path(new_path)
assert path == os.path.join(os.path.expanduser("~"), "EXPANDED", "path")
new_path = os.path.join("$HELLO_WORLD", "path")
assert path == new_path

# Now pretend the path exists
monkeypatch.setattr(os.path, "exists", alwaystrue)
path = resolve_path(new_path)
assert path == os.path.join(os.path.expanduser("~"), "EXPANDED", filename)

new_path = os.path.join("$HELLO_WORLD", filename)
path = resolve_path(new_path)
assert path == os.path.abspath(os.path.join("EXPANDED", "path"))
assert path == os.path.abspath(os.path.join("EXPANDED", filename))

0 comments on commit d7be9b2

Please sign in to comment.