diff --git a/pyhealth/datasets/base_dataset.py b/pyhealth/datasets/base_dataset.py index 476f44737..c4d8240f6 100644 --- a/pyhealth/datasets/base_dataset.py +++ b/pyhealth/datasets/base_dataset.py @@ -1,6 +1,7 @@ import logging import os import pickle +import posixpath from abc import ABC from pathlib import Path from typing import Dict, Iterator, Iterable, List, Optional, Any, Callable @@ -106,10 +107,21 @@ def is_url(path: str) -> bool: def clean_path(path: str) -> str: - """Clean a path string.""" + """Clean a path string. + + URL paths are normalised as POSIX paths on every OS; local paths are + expanded and resolved. + + Examples: + >>> from pyhealth.datasets.base_dataset import clean_path + >>> clean_path("https://example.org/a/b/../c.csv") + 'https://example.org/a/c.csv' + """ if is_url(path): parsed = urlparse(path) - cleaned_path = os.path.normpath(parsed.path) + # A URL path is POSIX whatever the host OS: os.path.normpath would + # rewrite the separators as backslashes on Windows. + cleaned_path = posixpath.normpath(parsed.path) # Rebuild the full URL return urlunparse(parsed._replace(path=cleaned_path)) else: diff --git a/tests/core/test_clean_path.py b/tests/core/test_clean_path.py new file mode 100644 index 000000000..5d0ea5072 --- /dev/null +++ b/tests/core/test_clean_path.py @@ -0,0 +1,45 @@ +import ntpath +import unittest +from unittest.mock import patch + +from pyhealth.datasets import base_dataset +from pyhealth.datasets.base_dataset import clean_path + +# (input URL, expected output). A URL path is POSIX whatever the host OS. +URL_CASES = [ + ( + "https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz", + "https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz", + ), + ( + "https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III//PATIENTS.csv", + "https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv", + ), + ( + "https://example.org/a/b/../c.csv", + "https://example.org/a/c.csv", + ), + ( + "s3://bucket/key/file.csv.gz", + "s3://bucket/key/file.csv.gz", + ), +] + + +class TestCleanPathURL(unittest.TestCase): + def test_url_path_is_normalised_as_posix(self): + for url, expected in URL_CASES: + with self.subTest(url=url): + self.assertEqual(clean_path(url), expected) + + def test_url_path_is_unaffected_by_windows_os_path(self): + # Reproduce the Windows behaviour on any OS: ntpath.normpath rewrites + # "/" as "\\", which must not leak into a URL. + with patch.object(base_dataset.os, "path", ntpath): + for url, expected in URL_CASES: + with self.subTest(url=url): + self.assertEqual(clean_path(url), expected) + + +if __name__ == "__main__": + unittest.main()