clean_path() normalises URL paths with os.path.normpath, breaking every URL-rooted dataset on Windows
Les mainteneurs répondent en général sous 2 jours
Une pull request liée a déjà été fusionnée.
- #1344 par @AxelNoun — fusionnée
Évaluation
- Difficulté
- 2/5
- Temps estimé
- 1-3 heures
- Accessibilité débutants
- 77/100
Piste de recherche
Commence par clean_path() dans pyhealth/datasets/base_dataset.py et examine le traitement des URL ainsi que les trois points d’appel décrits dans l’issue. Ajoute un test unitaire sans accès réseau pour les cas d’URL et de chemins locaux, puis exécute ce test. Le travail est terminé lorsque les séparateurs d’URL et les empreintes des sources du cache sont cohérents sur toutes les plateformes et que le test réussit.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
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:
_scan_table, line 847 — the main read path for every table.- the join path, line 867 — same, for joined tables.
_resolve_cache_dir, line 540 —_source_fingerprint(clean_path(f"{self.root}/{file_path}"))goes intokey["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:
- Would you like the fix to come with a unit test over
clean_pathfor the four cases above? It needs no network, unliketest_processor_schemas.py. line 113dates fromabc57230(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.
- Langage dominant
- Python
- Étoiles
- 1.7k
- Forks
- 803
- Merge moyen
- 4 j 18 h
- PR mergées (30 j)
- 58
Préparer son environnement
- Aucun Dockerfile ni fichier Docker Compose
- Aucun modèle de pull request
- Lire le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de sunlabuiuc/PyHealth
-
Difficulté 5/5 Plus d'une semaine Accessibilité débutants 25/100
sunlabuiuc/PyHealth#1357 · 1 commentaire ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 5/5 Plus d'une semaine Accessibilité débutants 35/100
sunlabuiuc/PyHealth#1346 ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 4/5 3-5 jours Accessibilité débutants 30/100
sunlabuiuc/PyHealth#1297 ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 4/5 3-5 jours Accessibilité débutants 22/100
sunlabuiuc/PyHealth#1295 · 1 commentaire ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 3/5 1-2 jours Accessibilité débutants 42/100
sunlabuiuc/PyHealth#1294 · 1 commentaire ·
Les mainteneurs répondent en général sous 2 jours
Toutes les issues de sunlabuiuc/PyHealth
Issues similaires
-
bug
Difficulté 2/5 1-3 heures Accessibilité débutants 86/100
NousResearch/hermes-agent#147104 ·
Les mainteneurs répondent en général sous 1 jour
-
[oblt-aw][autodoc] Document new required `shared-org-key` input for `obs-aw-dependency-review`Peut-être pris @github-actions l’a pris aujourd’hui. Ouverte
Difficulté 1/5 Moins d'une heure Accessibilité débutants 90/100
Les mainteneurs répondent en général sous 1 jour
-
bot-triaged module: inductor oncall: pt2 topic: fuzzer triaged
Difficulté 2/5 1-3 heures Accessibilité débutants 82/100
Les mainteneurs répondent en général sous 1 jour
-
getchaintips leaves out the active tip once a header is staged on it, where Core always lists itOuverte
Difficulté 2/5 1-3 heures Accessibilité débutants 88/100
btclib-org/btclib-node#2158 ·
Les mainteneurs répondent en général sous 1 jour
-
Difficulté 2/5 1-3 heures Accessibilité débutants 75/100
patrick-hudson/put.gd-desktop#979 · 1 commentaire ·
Les mainteneurs répondent en général sous 1 jour