Skip to content

clean_path() normalises URL paths with os.path.normpath, breaking every URL-rooted dataset on Windows #1333

Closed

Description

@AxelNoun

clean_path() normalises the path component of a URL with os.path.normpath, which on Windows rewrites every / as \:

# pyhealth/datasets/base_dataset.py
def clean_path(path: str) -> str:
    """Clean a path string."""
    if is_url(path):
        parsed = urlparse(path)
        cleaned_path = os.path.normpath(parsed.path)
        # Rebuild the full URL
        return urlunparse(parsed._replace(path=cleaned_path))

urlunparse then puts those backslashes back into the URL, so no dataset with a URL root can be read on Windows.

Reproduction

Checked on master at 4a173f7, Windows 11, Python 3.13.5.

>>> from pyhealth.datasets.base_dataset import clean_path
>>> clean_path("https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz")
'https://storage.googleapis.com/\\pyhealth\\Synthetic_MIMIC-III\\PATIENTS.csv.gz'

End to end, any URL-rooted dataset fails. The three tests in tests/core/test_processor_schemas.py, which build a MIMIC3Dataset on https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III, all fail the same way:

FileNotFoundError: Neither path exists:
  https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz or
  https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv
pyhealth\datasets\base_dataset.py:203: FileNotFoundError

The error surfaces in _csv_tsv_gz_path, but the mangling happens earlier, in clean_path.

Reach

clean_path has three call sites, and the bug is not limited to reading:

  1. _scan_table, line 847 — the main read path for every table.
  2. the join path, line 867 — same, for joined tables.
  3. _resolve_cache_dir, line 540 — _source_fingerprint(clean_path(f"{self.root}/{file_path}")) goes into key["sources"], which is hashed into the dataset's cache UUID.

The third one has a consequence beyond Windows being broken: for the same dataset, the cache key is platform-dependent. With the synthetic MIMIC-III root and tables=["patients"]:

platform source recorded in the key resulting dataset_uuid
Windows https://storage.googleapis.com/\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz 8cd7c9be-4dfd-5c81-a105-fdc4042b34dc
POSIX https://storage.googleapis.com/pyhealth/Synthetic_MIMIC-III/PATIENTS.csv.gz 6c4bdad7-c16e-5d84-b49f-54a704600a3d

So a cache directory built on Linux is not found by a Windows run of the same code, and vice versa. That matters for anyone sharing a cache across machines, and it is invisible: nothing reports a miss, the dataset is simply rebuilt under another UUID.

The same substitution also applies to non-HTTP schemes, since is_url only requires a scheme and a netloc: s3://bucket/key/file.csv.gz becomes s3://bucket/\key\file.csv.gz.

Fix

A URL path is always POSIX, so it should be normalised with posixpath, which behaves identically on every platform:

import posixpath

def clean_path(path: str) -> str:
    """Clean a path string."""
    if is_url(path):
        parsed = urlparse(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)
        return urlunparse(parsed._replace(path=cleaned_path))
    else:
        return str(Path(path).expanduser().resolve())

Verified on the same machine:

input current with posixpath
.../Synthetic_MIMIC-III/PATIENTS.csv.gz .../\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv.gz unchanged, correct
.../Synthetic_MIMIC-III//PATIENTS.csv .../\pyhealth\Synthetic_MIMIC-III\PATIENTS.csv .../Synthetic_MIMIC-III/PATIENTS.csv
https://example.org/a/b/../c.csv https://example.org/\a\c.csv https://example.org/a/c.csv
s3://bucket/key/file.csv.gz s3://bucket/\key\file.csv.gz unchanged, correct

Note that the fix changes the cache key of URL-rooted datasets on Windows (to the POSIX one, which is the correct and portable value), so existing Windows caches will be rebuilt once. That seems preferable to keeping them keyed on a malformed URL, but say so if you would rather handle it differently.

Two questions before I send a PR:

  1. Would you like the fix to come with a unit test over clean_path for the four cases above? It needs no network, unlike test_processor_schemas.py.
  2. line 113 dates from abc57230 (1 May 2025), so the behaviour has been there for a while, and CI apparently runs on Linux only. Would a Windows job on a small subset of the test suite be welcome, or is that out of scope here?

Happy to open the PR either way.

Activity

  1. solarsys commented on Oct 27, 2026

    @solarsys
    Collaborator

    @AxelNoun thanks, this is a clear diagnosis, and the posixpath fix is right. Please send the PR. On your questions:

    1. Unit test: yes, please add one over clean_path with the four cases from your table. It needs no network. You can show the Windows behaviour on any OS by patching the module's os.path with ntpath; on master that test fails.
    2. Windows CI job: that's worth discussing, but in a separate issue, since it adds CI time on every PR. Please keep this PR to the fix and the test.

    Rebuilding Windows caches once, under the portable key, is fine. Please mention it in the PR description.

  2. AxelNoun commented on Oct 28, 2026

    @AxelNoun
    ContributorAuthor

    Thanks @solarsys. I'm on it: the PR will contain the posixpath fix and a unit test over clean_path for the four cases, including a variant that patches the module's os.path with ntpath so the Windows behaviour is checked on any OS. I'll mention the one-time rebuild of Windows caches in the description and leave the Windows CI job for a separate issue.

  3. added a commit that references this issue on Oct 28, 2026
    45a8db9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions